Revert "feat(upgrades): implement v2.5.0 upgrade handler that modifies the stNIBI ERC20 and Bank Coin metadata in place" #2323
Conversation
…s the st…" This reverts commit 38c87ca.
📝 WalkthroughWalkthroughThis change removes the v2.5.0 upgrade handler and its associated test suite, deletes related code and references throughout the application, and standardizes the naming of the upgrade keeper field. Minor code cleanups and comment clarifications are also included, with no changes to core logic outside the upgrade removal. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant UpgradeHandler
participant EVM
participant Bank
App->>UpgradeHandler: (Previously) Register v2.5.0 handler
UpgradeHandler->>EVM: (Previously) Modify stNIBI ERC20 contract
UpgradeHandler->>Bank: (Previously) Update coin metadata
Note over App,UpgradeHandler: Now, v2.5.0 handler and flow are removed
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
x/evm/keeper/evm_state.go (1)
34-35: Comment clarification is helpful but could be more precise.The updated comment correctly emphasizes the composite primary key
(EthAddr + EthHash), but the phrase “because there's exactly one deployer and withdrawer” may confuse readers unfamiliar with the context. Consider rephrasing to clarify that the contract address uniquely identifies the contract’s storage namespace and the state key hash scopes individual entries, for example:// - primary key (PK): (EthAddr + EthHash). // The contract address identifies the contract namespace, and the state key hash distinguishes its storage slots.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md(0 hunks)app/app.go(2 hunks)app/keepers.go(4 hunks)app/keepers/all_keepers.go(0 hunks)app/upgrades.go(2 hunks)app/upgrades/v2_5_0/v2_5_0.go(0 hunks)app/upgrades/v2_5_0/v2_5_0_test.go(0 hunks)x/evm/evmtest/test_deps.go(0 hunks)x/evm/keeper/erc20.go(0 hunks)x/evm/keeper/evm_state.go(1 hunks)x/evm/keeper/funtoken_from_coin.go(1 hunks)
💤 Files with no reviewable changes (6)
- CHANGELOG.md
- app/keepers/all_keepers.go
- x/evm/keeper/erc20.go
- x/evm/evmtest/test_deps.go
- app/upgrades/v2_5_0/v2_5_0.go
- app/upgrades/v2_5_0/v2_5_0_test.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
x/evm/keeper/funtoken_from_coin.go (1)
x/evm/embeds/embeds.go (1)
SmartContract_ERC20MinterWithMetadataUpdates(68-71)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: integration-tests
- GitHub Check: e2e-evm
- GitHub Check: unit-tests
- GitHub Check: lint
🔇 Additional comments (8)
x/evm/keeper/funtoken_from_coin.go (1)
77-77: Formatting simplification is safe and clear.Consolidating the ABI.Pack call into a single line retains the exact same arguments and logic. Given the short parameter list, readability remains high, and no functional change is introduced.
app/upgrades.go (1)
76-76: LGTM: Consistent field naming convention applied.The renaming from
UpgradeKeepertoupgradeKeeperfollows Go naming conventions for unexported fields and is applied consistently across all upgrade keeper method calls.Also applies to: 87-87, 92-92
app/app.go (2)
315-315: LGTM: Dependency injection correctly updated.The depinject configuration properly targets the renamed
upgradeKeeperfield, maintaining the same initialization behavior.
495-495: LGTM: InitChainer usage updated consistently.The
SetModuleVersionMapcall correctly uses the renamedupgradeKeeperfield, preserving the module version initialization functionality.app/keepers.go (4)
34-34: LGTM: Import added for upgrade keeper.The import for
upgradekeeperpackage is correctly added to support the new field declaration.
74-74: LGTM: Upgrade keeper field properly added to AppKeepers.The
upgradeKeeperfield is correctly added as a private field to theAppKeepersstruct, following Go naming conventions and improving encapsulation by moving it fromPublicKeepers.
151-151: LGTM: IBC keeper initialization uses renamed field.The IBC keeper initialization correctly references the renamed
upgradeKeeperfield, maintaining the same functionality.
345-345: LGTM: Governance router setup updated consistently.The software upgrade proposal handler correctly uses the renamed
upgradeKeeperfield in the governance router configuration.
|
We won't have to revert this PR if we get #2324 merged, which is a hot-fix on top of the v2.5.0 upgrade handler |
Reverts #2315