feat(tokenfactory): burn native method #1832
Conversation
WalkthroughThis update introduces a new upgrade version Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
x/inflation/types/tx.pb.gois excluded by!**/*.pb.go,!**/*.pb.gox/inflation/types/tx.pb.gw.gois excluded by!**/*.pb.gw.go,!**/*.pb.gw.gox/tokenfactory/types/tx.pb.gois excluded by!**/*.pb.go,!**/*.pb.go
Files selected for processing (11)
- app/upgrades.go (1 hunks)
- app/upgrades/v1_2_0/constants.go (1 hunks)
- proto/nibiru/inflation/v1/tx.proto (1 hunks)
- proto/nibiru/tokenfactory/v1/tx.proto (2 hunks)
- x/inflation/keeper/msg_server.go (1 hunks)
- x/inflation/keeper/msg_server_test.go (1 hunks)
- x/tokenfactory/cli/tx.go (3 hunks)
- x/tokenfactory/keeper/keeper_test.go (1 hunks)
- x/tokenfactory/keeper/msg_server.go (1 hunks)
- x/tokenfactory/keeper/msg_server_test.go (1 hunks)
- x/tokenfactory/types/tx_msgs.go (7 hunks)
Additional Context Used
GitHub Check Runs (1)
break-check failure (2)
proto/nibiru/inflation/v1/tx.proto: [failure] 11-11:
Previously present RPC "Burn" on service "Msg" was deleted.
Additional comments not posted (9)
app/upgrades/v1_2_0/constants.go (1)
12-22: The upgrade configuration forv1.2.0is well-defined and follows the expected structure for defining upgrades in Cosmos SDK applications. It's important to ensure that the upgrade logic inRunMigrationsand theStoreUpgradesare thoroughly tested to prevent any issues during the upgrade process.app/upgrades.go (1)
10-15: Addingv1_2_0.Upgradeto theUpgradeslist correctly registers the new upgrade version. Ensure that the upgrade logic forv1_2_0is thoroughly tested, especially in scenarios where an upgrade fromv1_1_0directly tov1_2_0is performed, to ensure compatibility and smooth transition.x/tokenfactory/keeper/keeper_test.go (1)
64-65: The addition of theMsgBurnNativecase in theHandleMsgfunction correctly implements the handling of the new message type for burning native tokens. It's important to ensure comprehensive test coverage for this new functionality, including edge cases and error handling scenarios, to ensure robustness.proto/nibiru/tokenfactory/v1/tx.proto (1)
27-29: The addition of theBurnNativeRPC method in the tokenfactory module'stx.protofile correctly introduces the functionality to burn native tokens such as "unibi." Ensure that the implementation of this RPC method is thoroughly tested, including validation of the message fields and error handling, to ensure robustness and security.x/tokenfactory/cli/tx.go (1)
215-253: The addition of theCmdBurnNative()command inx/tokenfactory/cli/tx.gocorrectly implements the CLI command for burning native tokens. It's important to ensure that the command's help text and error messages are clear and informative to guide users effectively. Additionally, consider adding examples in the command's help text to demonstrate usage.x/tokenfactory/keeper/msg_server.go (1)
278-301: The implementation ofBurnNativecorrectly handles the burning of native tokens by interacting with the bank keeper to send and burn coins. However, there are a few areas for improvement and verification:
Error Handling and Logging: Ensure that all errors are logged appropriately for better traceability and debugging. Consider adding logging statements for significant steps within the function, especially before returning errors.
Performance Considerations: The current implementation is straightforward and does not introduce unnecessary complexity. However, always consider the impact of new features on the overall system performance, especially in high-throughput environments.
Security Considerations: The function relies on the
bankKeeperfor critical operations. Ensure that thebankKeeper's methods used here (SendCoinsFromAccountToModuleandBurnCoins) have their security implications reviewed, particularly regarding authorization checks and potential edge cases.Testing and Validation: It's crucial to have comprehensive tests covering various scenarios, including but not limited to successful burns, attempts to burn more tokens than available, and invalid input cases. If not already covered, enhance the test suite to include these scenarios.
Documentation: Ensure that the function and its parameters are well-documented, explaining the purpose, expected inputs, and any side effects. This is crucial for maintainability and for future developers who might work on this code.
Consider adding more detailed logging and error handling for better observability and security auditing. Also, ensure comprehensive testing around this new functionality.
x/tokenfactory/types/tx_msgs.go (2)
14-19: The implementation of interfaces for the new message types (MsgChangeAdmin,MsgUpdateModuleParams,MsgMint,MsgBurn,MsgSetDenomMetadata, andMsgBurnNative) is correctly done by ensuring they all satisfy thelegacytx.LegacyMsginterface. This is crucial for the messages to be processed correctly by the Cosmos SDK's transaction handling logic.The addition of these interfaces is necessary for the new message types to be properly integrated into the Cosmos SDK's transaction system. Good job on ensuring consistency with the SDK's requirements.
266-298: TheMsgBurnNativemessage type'sValidateBasicmethod correctly performs basic validation checks, such as ensuring the sender's address is valid and the coin to be burned is properly validated. This is crucial for preventing invalid or malicious transactions from being processed.The validation logic for
MsgBurnNativeis well-implemented, ensuring that only valid messages are processed. This is an essential step in maintaining the integrity of transactions within the system.x/tokenfactory/keeper/msg_server_test.go (1)
640-725: TheTestBurnNativefunction correctly implements test cases for theBurnNativefunctionality, covering scenarios such as successful burning, insufficient funds, and handling of nil messages. This comprehensive testing is crucial for ensuring the reliability and correctness of theBurnNativefunctionality.
Coverage and Scenarios: The test cases cover essential scenarios, which is good practice. Consider adding more edge cases if applicable, such as attempting to burn tokens of a non-existent denomination or tokens that the sender does not own.
Error Handling and Assertions: The use of
s.NoErrorands.Equalfor assertions is appropriate. Ensure that all possible error paths are tested and that assertions are made for all critical outcomes of the operations.Test Setup and Teardown: The setup and teardown processes are correctly handled using the
SetupMsgsandPreHook/PostHookmechanisms. This ensures a clean testing environment for each test case.The
TestBurnNativefunction is well-implemented, providing thorough testing for theBurnNativefunctionality. Continue to maintain high coverage and consider additional edge cases for even more robust testing.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional Context Used
Additional comments not posted (1)
CHANGELOG.md (1)
59-59: The entry for PR #1832 under "State Machine Breaking" might be misleading as it suggests a breaking change. However, adding a new feature like a burn method for native tokens typically doesn't constitute a breaking change unless it alters existing behavior or data structures in a way that would require intervention when upgrading. Please verify if this was indeed a breaking change or if it should be categorized differently, such as under "Features" or "Improvements".
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- x/tokenfactory/types/codec.go (1 hunks)
Additional Context Used
Additional comments not posted (1)
x/tokenfactory/types/codec.go (1)
37-37: The addition ofMsgBurnNativeto theRegisterInterfacesfunction is consistent with the Cosmos SDK's approach to registering message types. Ensure that all necessary integrations, including Amino JSON serialization and any other related changes, are completed throughout the system.
* refactor: move Burn rpc method to x/tokenfactory * Update tx_msgs.go * feat: add burn native method to tokenfactory msg server * feat: add cli cmd * fix: validate msg * feat: add v1.2.0 upgrade handler * Update CHANGELOG.md * fix: register codec for MsgBurnNative * fix: test --------- Co-authored-by: Unique-Divine <[email protected]>
* chore(perp): remove perp * chore(perp): remove perp protos * chore(stablecoin): remove stablecoin * chore(spot): remove spot * chore: use large ubuntu runner for goreleaser workflow * fix(ledger): bump the hid package to v0.9.2 which fixes ledger on new… (#1649) * fix(ledger): bump the hid package to v0.9.2 which fixes ledger on new macos versions * chore: update changelog * chore: update changelog * feat(wasmbinding)!: whitelisted stargate queries for QueryRequest::Stargate: auth, bank, gov, tokenfactory, epochs, inflation, oracle, sudo, devgas (#1646) * fix(tokenfactory)!: Fix bug in MsgBurn on total supply tracking * chore: rm stablecoin. How does this keep getting merged lol * test: add export statements for the gRPC query service descriptions in each module * feat(wasmbinding): whitelisted stargate queries for QueryRequest::Stargate * changelog * fix changelog * refactor!: make the epoch infos name consistent * docs,test(stargate_query): leave an in-depth explainer above the function * refactor: pR comments: earlier return + remove duplicate hardcoded paths * test: proto package may have more than 3 'parts'. Use len - 1 instead * docs: fix small documentation typos * chore: reset module consensus versions * fix(inflation): enable inflationary NIBI (#1655) * chore(epochs): add day epoch and remove 15min epoch from default genesis * chore(epochs): remove 15min epoch from default genesis * fix(inflation): inflate strategic reserves to sudo root account * fix(inflation): test inflation amounts * chore: update changelog * fix(keeper): inflation keeper instantiation * feat(oracle): edit oracle params msg (#1658) * chore: update changelog * Update CHANGELOG.md * chore(oracle): curate default oracle whitelist (#1659) * chore(oracle): curate default oracle whitelist * update changelog * chore(deps): Bump github.com/CosmWasm/wasmvm from 1.4.1 to 1.5.0 (#1657) Bumps [github.com/CosmWasm/wasmvm](https://github.com/CosmWasm/wasmvm) from 1.4.1 to 1.5.0. - [Release notes](https://github.com/CosmWasm/wasmvm/releases) - [Changelog](https://github.com/CosmWasm/wasmvm/blob/main/CHANGELOG.md) - [Commits](CosmWasm/wasmvm@v1.4.1...v1.5.0) --- updated-dependencies: - dependency-name: github.com/CosmWasm/wasmvm dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: Unique-Divine <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Unique Divine <[email protected]> * chore(deps): Bump google.golang.org/grpc from 1.58.3 to 1.59.0 (#1643) * chore(deps): Bump google.golang.org/grpc from 1.58.3 to 1.59.0 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.58.3 to 1.59.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.58.3...v1.59.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Updated changelog - dependabot --------- Signed-off-by: Unique-Divine <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Unique Divine <[email protected]> Co-authored-by: Unique-Divine <[email protected]> Co-authored-by: Unique-Divine <[email protected]> * chore(deps): Bump github.com/cosmos/ibc-go/v7 from 7.3.0 to 7.3.1 (#1647) * chore(oracle): remove bnb:usd from default whitelist * chore(epochs): add month default epoch * chore(wasm): bump wasmd dependency to v0.43.0 (#1660) * chore(wasm): bump wasmd dependency to v0.43.0 * Update CHANGELOG.md * fix: fix linter issues --------- Co-authored-by: Matthias <[email protected]> * fix(epochs): fix epoch tests broken by adding month epoch into default genesis * feat: update wasmd to v0.44.0 (#1666) * feat: update wasmd to v0.44.0 * chore: lint * chore(inflation)!: remove x/inflation module (#1667) * chore(inflation): remove x/inflation and inflation proto files * fix: wasmbinding imports * chore(inflation): remove inflation from app keepers * update changelog * feat(inflation)!: make inflation follow a polynomial distribution (#1670) * feat: make inflation follow a polynomial distribution * chore: changelog * fix: fix inflation test * fix: fix epoch/period confusion * chore: update changelog * feat: add inflation events detailed distribution (#1695) * feat: add inflation events detailed distribution * chore: changelog * fix: improve error handling * fix: broken unit test + make x/sudo safer by making blank genesis invalid * refactor: run gofumpt formatter --------- Co-authored-by: Unique-Divine <[email protected]> * fix(inflation): fix default inflation allocation params (#1688) * fix: fix default inflation allocation * chore: changelog * fix: fix tests --------- Co-authored-by: Unique Divine <[email protected]> * feat: add upgrade handler for inflation module (#1684) * upgrade handler * make lint * update changelog * fix changelog * use orderedModuleNames() function for upgrade --------- Co-authored-by: Unique Divine <[email protected]> * fix: upgrade handler for v1.1.0 (#1706) * fix: upgrade handler for v1.1.0 * chore: update changelog * refactor(inflation): make inflation disabled by default (#1712) * refactor(inflation): make inflation disabled by default * chore: update changelog * chore: bump librocksdb to v8.9.1 (#1778) Co-authored-by: Unique Divine <[email protected]> * chore: update changelog * ci: fix go mod * ci: remove go toolchain directive * feat: make inflation params a collection and add commands to update them (#1776) * feat: make inflation params a collection and add commands to update them * fix: more tests * chore: changelog * Update x/inflation/keeper/grpc_query.go Co-authored-by: Unique Divine <[email protected]> * Update x/inflation/keeper/grpc_query.go Co-authored-by: Unique Divine <[email protected]> * Update x/inflation/keeper/sudo.go Co-authored-by: Unique Divine <[email protected]> * fix: fix tests and improve function * fix: add test for queryserver --------- Co-authored-by: Unique Divine <[email protected]> * ci: bump grocksdb to 1.8.12 * fix: remove verify-dist-temp * ci: use new goreleaser image * ci: update make build commands * fix(inflation): fix first epoch's inflation distribution (#1786) * fix: fix inflation start bug * fix: fix off by 2 on inflation start * fix: make test more random * chore: changelog * fix: use sdkmath.Int instead of sdk.Int * comments + var names --------- Co-authored-by: Unique-Divine <[email protected]> * ci: update goreleaser config file * feat: upgrade handler for v1.0.1 * feat(inflation): add inflation tx cmds (#1795) * feat(inflation): add toggle-inflation cli command * feat(inflation): add edit-params command and update validate-basic * chore: update changelog * feat: uncomment amino registering on register legacy amino codec for inflation module (#1792) * uncomment amino registering on register legacy amino codec * add changelog entry --------- Co-authored-by: Unique-Divine <[email protected]> * feat: handle case where skip epoch not aligned with epoch passed (#1796) * fix(inflation): num skipped epochs tracking (#1797) * fix: fix bug on inflation * chore: changelog * fix: add bool to see wether inflation ever started * fix: epoch off by one error * chore: unwire epochs keeper from inflation keeper * fix: off by one tests --------- Co-authored-by: Kevin Yang <[email protected]> * chore: bump cosmos-sdk to v0.47.9 (#1808) * chore: bump cosmos-sdk to v0.47.9 * chore: update changelog * chore: add v1.0.2 upgrade handler * fix(inflation-docs): ① Document inflation + ② delete unused code + ③ fix CI (#1799) * refactor,docs(inflation): Document everything + delete unused code * changelog * feat(localnet): make perp and spot modules optional features * fix(e2e-wasm.yml-ci): Use consistent command runner * wip! move nibid to path after downlaoding release * wip! fix syntax error in localnet.sh * wip!: what version is running? * wip! try: simplify and use fresh build * wip! fix param space * wip!: fix source path in chaosnet and localnet * wip!: fix source path in chaosnet and localnet * wip!: fix source path in chaosnet and localnet * fix(localent.sh): missing prices in oralce genesis * wait a bit since localnet is fixed now * fix(deploy-wasm): using wrong binary name * ci: Runs well but needs a better name * ci: fix chaosnet build (#1806) * ci: fix chaosnet build * fix(scripts): use better current absolute path directory fn * refactor(e2e-localnet): remove unused script * fix(justfile): handle case where stop is called without anything running --------- Co-authored-by: Unique-Divine <[email protected]> * chore: bump cosmos-sdk to v0.47.9 (#1808) * chore: bump cosmos-sdk to v0.47.9 * chore: update changelog * feat: update default inflation rate (#1804) * feat: update default inflation rate * chore: changelog * fix: fix rounding issue * ci: remove go toolchain in go.mod file * ci: revert back to cosmos sdk v0.47.5 * ci: bump to cosmos-sdk v0.47.6 * ci: bump to cosmos-sdk v0.47.7 * ci: bump to cosmos-sdk v0.47.8 * ci: bump to cosmos-sdk v0.47.9 * ci: revert to cosmos sdk v0.47.5 * chore: bump cosmos-sdk to v0.47.10 * chore: add back v1.0.1 files * fix: add back v1.0.2 upgrade handler * fix: wasm ibc transaction panic (#1816) * fix wasm ibc transaction panic * include upgrade 1.0.3 * linter * update and fix changelog * feat: base64 decoder for cw3 wasm messages (#1731) * wip * feat: have a dirty funny looking solution * chore: changelog * fix: fix replace usage * Update cmd/nibid/cmd/decode_base64.go Co-authored-by: Kevin Yang <[email protected]> * fix: turns out InterfaceRegistry.Resolve does the trick * chore: changelog * fix: remove unused import --------- Co-authored-by: Kevin Yang <[email protected]> * refactor(decode-base64): clean code improvements and fn docs (#1754) * add date into changelog for release * chore: finalize v1.1.0 changelog * feat(inflation): add burn method (#1823) * feat(inflation): add burn method * Update CHANGELOG.md * chore: fix changelog (#1824) * test(inflation): add additional burn test cases (#1828) * test(inflation): add negative burn tests * test(inflation): add total supply check to burn tests * Update CHANGELOG.md * feat(tokenfactory): burn native method (#1832) * refactor: move Burn rpc method to x/tokenfactory * Update tx_msgs.go * feat: add burn native method to tokenfactory msg server * feat: add cli cmd * fix: validate msg * feat: add v1.2.0 upgrade handler * Update CHANGELOG.md * fix: register codec for MsgBurnNative * fix: test --------- Co-authored-by: Unique-Divine <[email protected]> * revert: remove x/inflation burn method * feat(ica): ICA Host / Controller integration (#1820) * ICA Host / Controller integration. Added upgrade constants. Added make format command * Fixed required message URLs * Code import fix * Fixed upgrade name * Fixed upgrade target * Changed version number * Update CHANGELOG.md --------- Co-authored-by: Jonathan Gimeno <[email protected]> Co-authored-by: Kevin Yang <[email protected]> * refactor(oracle): add oracle slashing events (#1859) * refactor(oracle): add oracle slashing events * Update CHANGELOG.md * fix(ica): add controller stack (#1864) * fix(ica): add controller stack * chore: update changelog * fix: linter issues * Update CHANGELOG.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(wasm)!: increase contract size limit to 3MB (#1906) * feat(wasm): increase contract size limit to 3MB * chore: update changelog * feat: add pebbledb support (#1818) * chore: make default db backend PebbleDB * chore: update changelog * chore: add v1.4.0 upgrade handler * feat(ibc): add wasm stack to ibc router (#1931) * feat(ibc): add wasm stack to ibc router * Update CHANGELOG.md * feat: add v1.5.0 upgrade handler * chore: update changelog * feat: add pebbledb support * chore: remove wasmbinding folder and nibiru/v1 dependency * fix: remove extra overrideWasmVariables * fix: upgrades * fix: oracle keeper * fix: epochs tests * feat: add v2.0.0 upgrade handler * linter * fix: upgrade handlers had wrong function type * Revert "feat: enable wasm light clients on IBC (08-wasm) (#2068)" This reverts commit ee5e29f. * fix: remove v2.1.0 upgrade handler * chore: update changelog for v2 EVM release * fix: revert testnet-1 chain id to 7210 * fix: revert wasmvm to v1.5.0 * feat(ibc): add back 08-wasm client * chore: minor refactors to resemble main branch --------- Signed-off-by: Unique-Divine <[email protected]> Signed-off-by: Unique-Divine <[email protected]> Co-authored-by: Unique Divine <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Unique-Divine <[email protected]> Co-authored-by: Unique-Divine <[email protected]> Co-authored-by: Matthias <[email protected]> Co-authored-by: Matthias <[email protected]> Co-authored-by: Jonathan Gimeno <[email protected]> Co-authored-by: Helder Moreira <[email protected]> Co-authored-by: Segfault <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Oleg Nikonychev <[email protected]>
Purpose / Abstract
x/tokenfactorymsg server. The tokenfactory module was chosen because it already has theburnerpermission.v1.2.0upgrade handlerlegacytx.LegacyMsginterface, which is required for amino signingSummary by CodeRabbit
New Features
v1_2_0for enhanced application functionality.Bug Fixes
BurnRPC method to streamline token operations and improve security.Documentation
tokenfactorymodule.Tests
Burnfunction, ensuring test suite relevance.BurnNativefunctionality, enhancing code reliability.