ci(localnet): Fix script, simplify, and test in CI. #1714
Conversation
WalkthroughThe recent updates streamline the build and configuration processes for the nibid binary, enhancing the automated testing environment. The build step now utilizes Changes
Related issues
Poem
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 X ? TipsChat with CodeRabbit Bot (
|
![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: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/unit-tests.yml (1 hunks)
- contrib/scripts/localnet.sh (4 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/unit-tests.yml
Additional comments: 9
contrib/scripts/localnet.sh (9)
112-113: The error handling after attempting to initialize the chain ID is correct, but it's important to ensure that the error message is informative enough for debugging purposes.
115-121: The consolidation of configuration commands into a single
nibid configcommand is a good simplification. However, it's important to verify that thenibidbinary accepts these configurations in the new format and that the changes are compatible with the rest of the system.125-125: Using
sedwith-iand$SEDOPTIONto edit theapp.tomlfile directly is a practical approach, but it's important to ensure that theapp.tomlfile structure is consistent across different environments to avoid potential issues with thesedcommand.141-141: The command to add a genesis account is updated. It's crucial to ensure that the
$BINARY keys show $val_key_name -acommand outputs the expected address format and that the genesis coins are correctly formatted and valid for the network.195-195: The addition of the
add-genesis-perp-marketcommand is a significant change. It's important to ensure that the command is executed with the correct parameters and that thecheck_failfunction is called correctly to handle potential failures.235-238: The
add_genesis_paramfunction is used to set oracle exchange rates. It's important to ensure that thejqcommand is correctly formatted and that thegenesis.jsonfile is properly updated without any syntax errors.245-246: The
gentxcommand is added to create a genesis transaction for the validator. It's crucial to verify that the amount specified (900000000unibi) is correct and that the--chain-idparameter matches the intended chain ID.252-253: The
collect-gentxscommand is used to collect genesis transactions into thegenesis.jsonfile. It's important to ensure that this command works as expected and that thegenesis.jsonfile is in a valid state after execution.263-263: The command to start the network is unchanged, but it's important to ensure that the
--pruning nothingflag is the desired behavior for the localnet environment.
| add_genesis_param '.app_state.sudo.sudoers.root = "'"$val_address"'"' | ||
|
|
||
| # hack for localnet since we don't have a pricefeeder yet | ||
| add_genesis_param '.app_state.oracle.exchange_rates[0].pair = "ubtc:unusd"' | ||
| add_genesis_param '.app_state.oracle.exchange_rates[0].pair = "ubtc:uuusd"' | ||
| add_genesis_param '.app_state.oracle.exchange_rates[0].exchange_rate = "'"$price_btc"'"' | ||
| add_genesis_param '.app_state.oracle.exchange_rates[1].pair = "ueth:unusd"' | ||
| add_genesis_param '.app_state.oracle.exchange_rates[1].pair = "ueth:uuusd"' | ||
| add_genesis_param '.app_state.oracle.exchange_rates[1].exchange_rate = "'"$price_eth"'"' | ||
|
|
||
| # ------------------------------------------------------------------------ | ||
| # Gentx | ||
| # ------------------------------------------------------------------------ | ||
|
|
||
| echo_info "Adding gentx validator..." | ||
| if $BINARY genesis gentx $val_key_name 900000000unibi --chain-id $CHAIN_ID; then | ||
| echo_success "Successfully added gentx" |
There was a problem hiding this comment.
Gentx has to occur after we add the address to the x/sudo genesis
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 1
CHANGELOG.md (1)
- 74-74: The changelog entry correctly summarizes the CI improvements and script fixes for
localnet.sh.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/sudo/types/event.pb.go
- x/sudo/types/state.pb.go
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- proto/nibiru/sudo/v1/event.proto (1 hunks)
- proto/nibiru/sudo/v1/state.proto (1 hunks)
Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- proto/nibiru/sudo/v1/state.proto
Additional comments: 1
proto/nibiru/sudo/v1/event.proto (1)
- 11-13: The addition of the
EventUpdateSudoersmessage with a non-nullablesudoersfield is correctly defined according to protobuf syntax and conventions.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/unit-tests.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/unit-tests.yml
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (4)
- .github/workflows/unit-tests.yml (1 hunks)
- CHANGELOG.md (1 hunks)
- contrib/bashlib.sh (2 hunks)
- justfile (3 hunks)
Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/unit-tests.yml
Additional comments: 5
justfile (5)
11-13: The
install-cleanrecipe is a good addition for ensuring a clean state before installation. However, ensure that thetempdirectory is indeed the correct directory to remove and that no other artifacts need cleaning.42-44: The
localnetrecipe correctly delegates to themake localnetcommand, which should handle the setup of the local network.57-62: The
test-chaosnetrecipe runs thechaosnet.shscript. Ensure that this script has proper error handling and cleanup mechanisms in place, as the justfile recipe does not provide any.76-77: The renaming of
release-testtotest-releaseis not shown in the hunks, but it aligns with the naming convention of other test recipes, which is a good practice for consistency.39-66: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [8-78]
The changes to the
justfilealign with the PR objectives of fixing, simplifying, and enhancing the reliability of thelocalnet.shscript, as well as ensuring its functionality is tested during CI.
Purpose
localnet.shscript is broken. This fixes it.Summary by CodeRabbit
Refactor
Chores