build: artifacts + just tidy + docs + fix nibiru-std proto import #117
Conversation
commit 2296e6f1e91db857318632089aa2cf888a4ce8ae Merge: c14558d dd90c9a Author: Unique-Divine <[email protected]> Date: Wed Jan 10 21:25:41 2024 -0600 Merge branch 'main' into realu/std commit c14558de1b80b739701ff3b146355ea815327192 Author: Unique-Divine <[email protected]> Date: Wed Jan 10 15:43:00 2024 -0600 quick save wip! commit dd90c9a Merge: bd792e4 902b788 Author: Jonathan Gimeno <[email protected]> Date: Fri Jan 5 19:00:52 2024 +0100 Merge pull request #111 from NibiruChain/feat/some-refactors-airdrop add some refactors to airdrop contract commit 902b788 Author: Jonathan Gimeno <[email protected]> Date: Fri Jan 5 17:41:16 2024 +0100 remove unused imports commit 0f337d1 Author: Jonathan Gimeno <[email protected]> Date: Fri Jan 5 17:39:21 2024 +0100 it fails when there are not enough funds commit 1d42220 Author: Jonathan Gimeno <[email protected]> Date: Fri Jan 5 17:31:57 2024 +0100 add some refactors, part 1 commit bd792e4 Author: Kevin Yang <[email protected]> Date: Wed Jan 3 14:22:23 2024 -0800 feat: initial airdrop contract (#110) * feat: initial airdrop contract * create cargo config * test: instantiate message * remove owner from instantiate msg * test: query functions * test: execute functions * feat: add manager list * feat: implement manager list * Create README.md * Update contracts/airdrop/src/contract.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update contracts/airdrop/src/contract.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update contracts/airdrop/src/contract.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update contracts/airdrop/src/contract.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix error messages --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> commit 88ae9fd Author: Matthias <[email protected]> Date: Wed Jan 3 15:32:33 2024 +0100 doc: Add cookbook for live smart contracts (#109)
WalkthroughThe changes across the codebase focus on refactoring for readability and maintainability. They include the removal of redundant 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 (
|
Codecov ReportAttention:
Additional details and impacted files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (13)
- Cargo.lock
- Cargo.toml
- artifacts/airdrop.wasm
- artifacts/cw3_flex_multisig.wasm
- artifacts/incentives.wasm
- artifacts/lockup.wasm
- artifacts/nibi_stargate.wasm
- artifacts/nibi_stargate_perp.wasm
- artifacts/nusd_valuator.wasm
- artifacts/pricefeed.wasm
- artifacts/shifter.wasm
- artifacts/token_vesting.wasm
- contracts/airdrop/Cargo.toml
Files selected for processing (24)
- artifacts/checksums.txt (1 hunks)
- contracts/airdrop/src/contract.rs (10 hunks)
- contracts/airdrop/src/msg.rs (2 hunks)
- contracts/airdrop/src/state.rs (1 hunks)
- contracts/airdrop/src/tests/execute/claim.rs (4 hunks)
- contracts/airdrop/src/tests/execute/mod.rs (1 hunks)
- contracts/airdrop/src/tests/execute/reward_users.rs (8 hunks)
- contracts/airdrop/src/tests/execute/withdraw.rs (4 hunks)
- contracts/airdrop/src/tests/instantiate.rs (4 hunks)
- contracts/airdrop/src/tests/mod.rs (1 hunks)
- contracts/airdrop/src/tests/query/campaign.rs (3 hunks)
- contracts/airdrop/src/tests/query/mod.rs (1 hunks)
- contracts/airdrop/src/tests/query/user_pool.rs (2 hunks)
- nibiru-std/src/math.rs (2 hunks)
- nibiru-std/src/proto/buf/nibiru.inflation.v1.rs (1 hunks)
- nibiru-std/src/proto/buf/nibiru.oracle.v1.rs (14 hunks)
- nibiru-std/src/proto/buf/nibiru.perp.v2.rs (5 hunks)
- nibiru-std/src/proto/buf/nibiru.sudo.v1.rs (1 hunks)
- nibiru-std/src/proto/type_url_nibiru.rs (2 hunks)
- packages/bash-rs/src/bash.rs (6 hunks)
- packages/bash-rs/src/errors.rs (1 hunks)
- packages/core-controller/src/contract.rs (1 hunks)
- packages/core-controller/src/msgs.rs (1 hunks)
- scripts/wasm_out.rs (1 hunks)
Files skipped from review due to trivial changes (10)
- contracts/airdrop/src/msg.rs
- contracts/airdrop/src/state.rs
- contracts/airdrop/src/tests/execute/mod.rs
- contracts/airdrop/src/tests/execute/reward_users.rs
- contracts/airdrop/src/tests/execute/withdraw.rs
- contracts/airdrop/src/tests/instantiate.rs
- contracts/airdrop/src/tests/mod.rs
- contracts/airdrop/src/tests/query/campaign.rs
- contracts/airdrop/src/tests/query/mod.rs
- nibiru-std/src/proto/buf/nibiru.sudo.v1.rs
Additional comments: 37
packages/bash-rs/src/errors.rs (1)
- 16-17: The addition of the
WhichBinNotPresenterror variant is a good practice for providing more descriptive error messages related to binary presence checks. This should help with debugging and user error messages.artifacts/checksums.txt (1)
- 1-11: Checksums for compiled wasm files are typically used to verify the integrity of the files. It's important to ensure that these checksums are updated whenever the corresponding wasm files are changed to maintain integrity checks.
contracts/airdrop/src/tests/query/user_pool.rs (2)
20-23: The formatting changes to the
managersvector improve readability by placing each address on a separate line.69-72: The assertion has been updated to directly compare the result of
query_user_rewardwith the expected error. This is a more direct and clear way to write the test.contracts/airdrop/src/tests/execute/claim.rs (3)
3-3: The import statement for
USER_REWARDShas been simplified by removing unnecessary braces. This is a minor but positive change for code cleanliness.21-24: The
managersvector declaration has been formatted for better readability, similar to the changes inuser_pool.rs.57-57: The assertion related to
USER_REWARDShas been simplified to check the absence of a user's rewards after a claim. This makes the test more concise and clear.packages/core-controller/src/msgs.rs (1)
- 22-22: The renaming of the
InsuranceFundWithdrawvariant toWithdrawPerpFundin theExecuteMsgenum aligns with the changes in contract execution logic mentioned in the PR overview. This change should be verified across the codebase to ensure that all references to this enum variant have been updated accordingly.Verification successful
The verification process has confirmed that the
InsuranceFundWithdrawvariant has been successfully renamed toWithdrawPerpFundin theExecuteMsgenum and that the new variant name is being used correctly in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old enum variant name to ensure it's no longer used. rg --type rust "InsuranceFundWithdraw" # Search for the new enum variant name to ensure it's used correctly. rg --type rust "WithdrawPerpFund"Length of output: 342
packages/bash-rs/src/bash.rs (6)
12-13: Changing the parameter type from
Stringto&strfor thebash_codeparameter in therun_bashfunction is a good practice as it avoids unnecessary cloning and can improve performance.35-35: The
build_bash_cmdfunction has also been updated to take a string slice, which is consistent with the changes made torun_bash.45-45: The
run_bash_and_printfunction now correctly takes a string slice, aligning with the other function signatures and avoiding unnecessary cloning.100-102: The
BashOutputstruct'scmdfield has been changed to take aStringinstead of a string slice. This change is appropriate since the struct needs to own the data it contains.159-166: The addition of the
which_ok_assertfunction is a useful utility for ensuring that a binary is present in the PATH, and it properly uses the newWhichBinNotPresenterror variant.169-176: The
run_bash_multifunction is a new addition that allows running multiple bash commands and collecting their outputs. This can be useful for scripting multiple steps in a single function call.nibiru-std/src/proto/buf/nibiru.inflation.v1.rs (1)
- 2-12: The addition of the
EventInflationDistributionstruct with fieldsstaking_rewards,strategic_reserve, andcommunity_poolis a clear way to represent the distribution of inflation. This should make it easier to work with inflation events in the codebase.contracts/airdrop/src/contract.rs (6)
49-51: The removal of the explicit
returnkeyword in theinstantiatefunction is a Rust idiom that favors implicit returns for the last expression in a block, making the code more idiomatic and concise.98-103: The addition of comments explaining the requirements and actions in the
reward_usersfunction is a good practice for maintainability, making the code easier to understand for future developers.160-163: The comments added to the
claimfunction provide a clear explanation of its behavior, which is beneficial for code readability and maintainability.189-192: The comments in the
withdrawfunction are descriptive and helpful for understanding the purpose and constraints of the function.242-243: The refactoring of error handling in the
query_campaignfunction to provide a more specific error message is a good improvement for debugging and error tracing.253-255: Similarly, the refactoring in the
query_user_rewardfunction to provide a specific error message when a user reward does not exist is a good practice for error clarity.scripts/wasm_out.rs (1)
- 97-97: Passing a reference to the
cmdvariable in therun_bash_and_printfunction call is consistent with the changes made in thebash-rspackage and avoids unnecessary cloning.packages/core-controller/src/contract.rs (2)
60-63: The match arms for
AddMember,RemoveMember, andChangeAdminin theexecutefunction appear to be correctly implemented with proper error handling and state changes. These changes align with the PR's objectives to improve maintainability and readability.60-63: The
can_executefunction is well-implemented, providing a clear and concise way to determine if the sender has the necessary permissions to perform certain actions. This function is a good example of encapsulating permission logic in a single place, which aids in maintainability and readability.nibiru-std/src/math.rs (3)
54-54: The addition of the return statement
return *self;within theelse ifblock of theaddmethod in theDecimalExtimplementation is correct. This ensures that if the otherDecimalExtis zero, the method returns the current instance without modification, which is the expected behavior for adding zero to a number.54-54: The removal of the
blank_testfunction from thetest_sign_decmodule is not visible in the provided code. If this function was not serving any purpose or was a placeholder without any assertions, its removal is justified to keep the test suite clean and focused on actual test cases.54-54: The implementations for mathematical operations and conversions in the
DecimalExtandSdkDecstructs are well-structured and include comprehensive error handling. This contributes to the robustness and maintainability of the mathematical utilities provided by thenibiru-stdcrate.nibiru-std/src/proto/type_url_nibiru.rs (2)
72-75: The implementation of the
Nametrait fornibiru::oracle::MsgEditOracleParamslooks correct and follows the established pattern in the file.269-276: The implementation of the
Nametrait fornibiru::perp::MsgWithdrawFromPerpFundandnibiru::perp::MsgCloseMarketis consistent with the rest of the file and follows the established pattern.nibiru-std/src/proto/buf/nibiru.oracle.v1.rs (2)
7-11: The
prostattributes for theParamsstruct fields have been updated to remove spaces around the equal sign. This change is consistent and improves the readability of the code.483-515: New message types
MsgEditOracleParamsandMsgEditOracleParamsResponsehave been added. Ensure that these new message types are integrated correctly with the rest of the system and that any necessary handlers or logic for these messages have been implemented.nibiru-std/src/proto/buf/nibiru.perp.v2.rs (6)
58-61: The addition of the
oracle_pairfield to theMarketstruct is consistent with the summary provided. This field is important for associating a market with its corresponding oracle pair, which is likely used for price referencing.906-906: The comment for
MsgChangeCollateralDenomindicates that it is only callable by sudoers, which is a security measure to ensure that only authorized users can change the collateral denomination. This is a good practice for administrative operations that could have a significant impact on the system.952-953: The
MsgShiftPegMultipliermessage is marked as[SUDO], indicating that it should only be callable by sudoers. This is another administrative operation that can impact the trading system significantly, and it is good to see that it is restricted to authorized users.970-971: Similarly,
MsgShiftSwapInvariantis also marked as[SUDO], which is appropriate for an operation that can alter the trading dynamics of the platform.988-1006: The
MsgWithdrawFromPerpFundmessage allows for withdrawal from the perpetual fund. It includes an optionaldenomfield for cases where assets other than the default (presumably NUSD) are to be withdrawn. This flexibility is good, but it's important to ensure that the implementation checks for valid denominations and permissions, especially since it's marked as[SUDO].1009-1022: The
MsgCloseMarketmessage is intended for administrative use to close a market. It's crucial that the implementation of this message includes proper authorization checks to prevent unauthorized closures.
| const NAME: &'static str = "MsgEditOracleParams"; | ||
| const PACKAGE: &'static str = PACKAGE_ORACLE; | ||
| } | ||
|
|
There was a problem hiding this comment.
There is a TODO comment about a type that exists but was not exported by the protos. It's important to address this TODO to ensure that all necessary types are properly exported and available for use.
| } | ||
|
|
||
| // ORACLE tx msg | ||
|
|
||
| impl Name for nibiru::oracle::MsgEditOracleParams { | ||
| const NAME: &'static str = "MsgEditOracleParams"; | ||
| const PACKAGE: &'static str = PACKAGE_ORACLE; | ||
| } | ||
|
|
||
| // ORACLE query | ||
|
|
||
| impl Name for nibiru::oracle::QueryExchangeRateRequest { |
There was a problem hiding this comment.
Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [413-415]
The test cases in the module seem to cover the Name trait implementations for the tokenfactory messages. Ensure that similar tests are implemented for the newly added Name trait implementations to maintain thorough test coverage.
| ExecuteMsg::WithdrawPerpFund { amount, to } => { | ||
| check_member(check)?; | ||
| todo!(); | ||
| let _cw_msg = ExecuteMsg::InsuranceFundWithdraw { amount, to }; | ||
| let _cw_msg = ExecuteMsg::WithdrawPerpFund { amount, to }; |
There was a problem hiding this comment.
The execute function has been updated to change the match arm from InsuranceFundWithdraw to WithdrawPerpFund. The variable _cw_msg is also updated accordingly. However, the actual logic for handling the WithdrawPerpFund message is marked as todo!(), indicating that the implementation is incomplete. It's important to ensure that the logic for this match arm is implemented before merging the PR.
Additionally, the commented-out code should either be removed or implemented if it's meant to be part of the final version. Leaving commented-out code can lead to confusion and clutter in the codebase.
ExecuteMsg::WithdrawPerpFund { amount, to } => {
check_member(check)?;
todo!();
let _cw_msg = ExecuteMsg::WithdrawPerpFund { amount, to };
// let res = Response::new()
// .add_message(cw_msg)
// .add_attributes(vec![attr("action", "insurance_fund_withdraw")]);
// Ok(res)
}The match arms for SetMarketEnabled, CreateMarket, and EditOracleParams in the execute function are placeholders with todo!() and commented-out code. This suggests that the functionality for these cases is not yet implemented. It's crucial to complete the implementation for these cases or remove them if they are not intended for the current scope of work.
Leaving todo!() in the code that is merged into the main branch can lead to runtime panics if these code paths are executed. Ensure that the implementation is completed or feature-flagged if it's work in progress.
The tests in the tests module cover various functionalities such as contract instantiation, admin checks, and member management. However, there are no tests for the new WithdrawPerpFund functionality or other new features introduced in this PR. It's important to add comprehensive tests for all new functionalities to ensure they work as expected and to prevent regressions in the future.
Summary by CodeRabbit
New Features
EventInflationDistributionstruct to represent NIBI token emissions.Improvements
Bug Fixes
DecimalExtimplementation to ensure correct behavior.Documentation
Refactor
executefunction to reflect the renaming of fund withdrawal operations.Style