Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve naming inconsistencies and update configurations #11

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

elix1er
Copy link
Contributor

@elix1er elix1er commented Jul 8, 2024

Description:

This PR addresses naming inconsistencies across the project and updates various configurations:

  • Standardize binary name
  • Unify chain IDs
  • Consolidate token denomination
  • Update Dockerfile to reflect standardized naming
  • Modify scripts, tests, and documentation for consistency
  • Upgrade CW20 contract version to v2.0.0
  • Add new exclusions to .gitignore
  • Update README and statesync instructions for testnet

Key changes:

  • .gitignore: Add new exclusions for test and contract files
  • Dockerfile: Update binary name and build flags
  • scripts/: Modify all scripts to use standardized binary and token names
  • testnets/: Update README and statesync instructions
  • Various other files: Replace inconsistent naming with standardized terms

This standardization effort ensures consistency across the codebase, improves project coherence, and prepares for the upcoming testnet launch.

Please review before merge - ./scripts/**.sh tests not yet working, running due to:

File at mytestnet/config/genesis.json is a valid genesis file

ending up with

6:54PM ERR CONSENSUS FAILURE!!! err="runtime error: invalid memory address or nil pointer dereference" module=consensus stack="goroutine 202 [running]:\nruntime/debug.Stack()\n\truntime/debug/stack.go:24 +0x64\ngithub.com/cometbft/cometbft/consensus.(*State).receiveRoutine.func2()\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:801 +0x44\npanic({0x1066cfe20?, 0x108866d50?})\n\truntime/panic.go:770 +0x124\ncosmossdk.io/store/rootmulti.(*Store).GetKVStore(0x14002048200, {0x0, 0x0})\n\tcosmossdk.io/store@v1.1.0/rootmulti/store.go:652 +0x24c\ngithub.com/cosmos/cosmos-sdk/types.Context.KVStore({{0x106c2c408, 0x108a7a0c0}, {0x106c49c80, 0x14002048200}, {{0x0, 0x0}, {0x0, 0x0}, 0x0, {0x0, ...}, ...}, ...}, ...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/types/context.go:337 +0x3c\ngithub.com/cosmos/cosmos-sdk/x/params/types.Subspace.kvStore(...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/x/params/types/subspace.go:77\ngithub.com/cosmos/cosmos-sdk/x/params/types.Subspace.Has({{_, _}, _, {_, _}, {_, _}, {_, _, _}, ...}, ...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/x/params/types/subspace.go:156 +0xac\ngithub.com/cosmos/cosmos-sdk/baseapp.GetConsensusParams({{0x106c2c408, 0x108a7a0c0}, {0x106c49c80, 0x14002048200}, {{0x0, 0x0}, {0x0, 0x0}, 0x0, {0x0, ...}, ...}, ...}, ...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/baseapp/params_legacy.go:117 +0x90\ngithub.com/cosmos/cosmos-sdk/baseapp.MigrateParams({{0x106c2c408, 0x108a7a0c0}, {0x106c49c80, 0x14002048200}, {{0x0, 0x0}, {0x0, 0x0}, 0x0, {0x0, ...}, ...}, ...}, ...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/baseapp/params_legacy.go:142 +0x58\ngithub.com/HyperiaNetwork/hyperia/app.(*HyperiaApp).FinalizeBlock.func1()\n\tgithub.com/HyperiaNetwork/hyperia/app/app.go:1077 +0x350\nsync.(*Once).doSlow(0x140026ca328?, 0x10303494c?)\n\tsync/once.go:74 +0x100\nsync.(*Once).Do(...)\n\tsync/once.go:65\ngithub.com/HyperiaNetwork/hyperia/app.(*HyperiaApp).FinalizeBlock(0x14002abc008, 0x140026c3e00)\n\tgithub.com/HyperiaNetwork/hyperia/app/app.go:1069 +0x58\ngithub.com/cosmos/cosmos-sdk/server.cometABCIWrapper.FinalizeBlock(...)\n\tgithub.com/cosmos/cosmos-sdk@v0.50.7/server/cmt_abci.go:44\ngithub.com/cometbft/cometbft/abci/client.(*localClient).FinalizeBlock(0x10304f620?, {0x106c2ce50?, 0x108a7a0c0?}, 0x111128180?)\n\tgithub.com/cometbft/cometbft@v0.38.9/abci/client/local_client.go:185 +0xe8\ngithub.com/cometbft/cometbft/proxy.(*appConnConsensus).FinalizeBlock(0x14000f55128, {0x106c2ce50, 0x108a7a0c0}, 0x140026c3e00)\n\tgithub.com/cometbft/cometbft@v0.38.9/proxy/app_conn.go:104 +0x130\ngithub.com/cometbft/cometbft/state.(*BlockExecutor).applyBlock(_, {{{0xb, 0x0}, {0x14001eacafa, 0x6}}, {0x14001eacb10, 0x7}, 0x1, 0x0, {{0x0, ...}, ...}, ...}, ...)\n\tgithub.com/cometbft/cometbft@v0.38.9/state/execution.go:224 +0x410\ngithub.com/cometbft/cometbft/state.(*BlockExecutor).ApplyVerifiedBlock(...)\n\tgithub.com/cometbft/cometbft@v0.38.9/state/execution.go:202\ngithub.com/cometbft/cometbft/consensus.(*State).finalizeCommit(0x1400155b508, 0x1)\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:1772 +0x97c\ngithub.com/cometbft/cometbft/consensus.(*State).tryFinalizeCommit(0x1400155b508, 0x1)\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:1682 +0x26c\ngithub.com/cometbft/cometbft/consensus.(*State).enterCommit.func1()\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:1617 +0x8c\ngithub.com/cometbft/cometbft/consensus.(*State).enterCommit(0x1400155b508, 0x1, 0x0)\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:1655 +0xac4\ngithub.com/cometbft/cometbft/consensus.(*State).addVote(0x1400155b508, 0x14000546270, {0x0, 0x0})\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:2335 +0x1858\ngithub.com/cometbft/cometbft/consensus.(*State).tryAddVote(0x1400155b508, 0x14000546270, {0x0?, 0x103098960?})\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:2067 +0x28\ngithub.com/cometbft/cometbft/consensus.(*State).handleMsg(0x1400155b508, {{0x106bf6060, 0x14000c76180}, {0x0, 0x0}})\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:929 +0x300\ngithub.com/cometbft/cometbft/consensus.(*State).receiveRoutine(0x1400155b508, 0x0)\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:856 +0x370\ncreated by github.com/cometbft/cometbft/consensus.(*State).OnStart in goroutine 1\n\tgithub.com/cometbft/cometbft@v0.38.9/consensus/state.go:398 +0xf0\n"

Summary by CodeRabbit

  • New Features

    • Transition from the "Eve" network to the "Hyperia" network with updated commands, paths, and configurations.
    • Updated testnet documentation to reflect the new network setup.
  • Bug Fixes

    • Improved error handling and logging in the FinalizeBlock method.
  • Style

    • Standardized token denominations to "uhype" across scripts and tests.
  • Chores

    • Added specific files and directories to .gitignore for improved git management.
    • Updated Dockerfile to reflect new binary name and version details.

Copy link
Contributor

coderabbitai bot commented Jul 8, 2024

Walkthrough

The updates transition from the Eve network (eved) to the Hyperia network (hyperiad). Changes include adjusting binary names, chain IDs, and denominations across Dockerfiles, scripts, test files, and documentation. The denominational change from uwhale to uhype impacts configuration, scripting, and testing processes. Additionally, versioning and logging related updates ensure proper execution and consensus parameter handling within the HyperiaApp struct.

Changes

File(s) Change Summary
.gitignore Added screenlog.0, contract_tests/*, and scripts/wasm/*.wasm to ignore list.
Dockerfile Updated binary name, versioning ldflags, and ENTRYPOINT to hyperiad.
scripts/run-node.sh Renamed process from eved to hyperiad, changed denomination to uhype, and updated genesis file parameters.
.../tests/alliance/delegate.sh Updated variable names and token denominations from UWHALE_DENOM to UHYPE_DENOM.
.../tests/init-test-framework.sh Updated binary name to hyperiad, chain IDs to hype-1/hype-2, denominational changes, and genesis accounts adjustments.
scripts/wasm/* (get_contract, ...) Updated version to v2.0.0, changed binary references to hyperiad, updated chain IDs to local-hype, and modified fee parameters.
testnets/README.md, statesync.md Updated references from eved to hyperiad, adjusted commands, paths, chain IDs, and denominations for the Hyperia Network Testnet documentation.
app/ante/ante_test.go, suite.go Changed denomination references from uhyperia to uhype and updated fee calculations accordingly.
app/app.go Enhanced FinalizeBlock method for consensus parameter migration, added error handling, logging, and initialization checks for ConsensusParamsKeeper.
scripts/gov_upload_code.sh Updated binary path, chain ID, denomination for transactions, and corresponding echo messages regarding denomination.

Poem

On Hyperia's tide, we embark anew,
From eved we bid adieu.
With hyperiad reign and uhype in tow,
In scripts and nodes, the changes flow.
A network reborn, precise and bright,
Here's to smooth sailing, day and night! 🚀💻


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@elix1er
Copy link
Contributor Author

elix1er commented Jul 8, 2024

@coderabbitai review

"title": "Create an Alliance!",
"summary": "Source Code Version https://github.com/terra-money/core"
}' > $CHAIN_DIR/create-alliance.json


echo "Creating an alliance with the denom $IBC_DENOM"

PROPOSAL_HEIGHT=$($BINARY tx gov submit-proposal $CHAIN_DIR/create-alliance.json --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --node tcp://localhost:26657 -o json --keyring-backend test --fees 600000$UWHALE_DENOM --gas 500000 -y -o json | jq -r '.height')
PROPOSAL_HEIGHT=$($BINARY tx gov submit-proposal $CHAIN_DIR/create-alliance.json --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --node tcp://localhost:26657 -o json --keyring-backend test --fees 600000$UHYPE_DENOM --gas 500000 -y -o json | jq -r '.height')

Check notice

Code scanning / devskim

Accessing localhost could indicate debug code, or could hinder scaling. Note test

Do not leave debug code in production
sleep 3


PROPOSAL_ID=$($BINARY query gov proposals --home $CHAIN_DIR/test-2 --count-total --node tcp://localhost:26657 -o json --output json --chain-id=test-2 | jq .proposals[-1].id -r)


VOTE_RES=$($BINARY tx gov vote $PROPOSAL_ID yes --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test --fees 60000$UWHALE_DENOM --chain-id=test-2 --node tcp://localhost:26657 -o json -y)
VOTE_RES=$($BINARY tx gov vote $PROPOSAL_ID yes --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test --fees 60000$UHYPE_DENOM --chain-id=test-2 --node tcp://localhost:26657 -o json -y)

Check notice

Code scanning / devskim

Accessing localhost could indicate debug code, or could hinder scaling. Note test

Do not leave debug code in production
VAL_WALLET_1=$($BINARY keys show val1 -a --keyring-backend test --home $CHAIN_DIR/test-1)
VAL_WALLET_2=$($BINARY keys show val2 -a --keyring-backend test --home $CHAIN_DIR/test-2)

echo "Sending tokens from validator wallet on test-1 to validator wallet on test-2"
IBC_TRANSFER=$($BINARY tx ibc-transfer transfer transfer channel-0 $VAL_WALLET_2 $AMOUNT_TO_DELEGATE$UWHALE_DENOM --chain-id test-1 --from $VAL_WALLET_1 --home $CHAIN_DIR/test-1 --fees 60000$UWHALE_DENOM --node tcp://localhost:16657 --keyring-backend test -y -o json | jq -r '.raw_log' )
IBC_TRANSFER=$($BINARY tx ibc-transfer transfer transfer channel-0 $VAL_WALLET_2 $AMOUNT_TO_DELEGATE$UHYPE_DENOM --chain-id test-1 --from $VAL_WALLET_1 --home $CHAIN_DIR/test-1 --fees 60000$UHYPE_DENOM --node tcp://localhost:16657 --keyring-backend test -y -o json | jq -r '.raw_log' )

Check notice

Code scanning / devskim

Accessing localhost could indicate debug code, or could hinder scaling. Note test

Do not leave debug code in production
Copy link
Contributor

coderabbitai bot commented Jul 8, 2024

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@elix1er elix1er marked this pull request as ready for review July 8, 2024 12:12
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ee3ac8d and 9410b57.

Files selected for processing (11)
  • .gitignore (2 hunks)
  • Dockerfile (2 hunks)
  • scripts/run-node.sh (4 hunks)
  • scripts/tests/alliance/delegate.sh (5 hunks)
  • scripts/tests/init-test-framework.sh (3 hunks)
  • scripts/wasm/get_contract.sh (1 hunks)
  • scripts/wasm/instantiate_cw20.sh (1 hunks)
  • scripts/wasm/test_wasm.sh (1 hunks)
  • scripts/wasm/upload_code.sh (1 hunks)
  • testnets/README.md (3 hunks)
  • testnets/statesync.md (3 hunks)
Files skipped from review due to trivial changes (6)
  • .gitignore
  • scripts/run-node.sh
  • scripts/wasm/get_contract.sh
  • scripts/wasm/instantiate_cw20.sh
  • scripts/wasm/test_wasm.sh
  • testnets/statesync.md
Additional context used
Shellcheck
scripts/wasm/upload_code.sh

[warning] 9-9: DENOM appears unused. Verify use (or export if used externally).

(SC2034)

scripts/tests/alliance/delegate.sh

[warning] 86-86: DELEGATE_RES appears unused. Verify use (or export if used externally).

(SC2034)

GitHub Check: devskim
scripts/tests/alliance/delegate.sh

[notice] 18-18: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production


[notice] 64-64: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production


[notice] 71-71: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production

Additional comments not posted (27)
scripts/wasm/upload_code.sh (2)

3-3: Change approved: Updated binary name.

The binary name has been correctly updated to hyperiad to standardize naming.


5-5: Change approved: Updated chain ID.

The chain ID has been correctly updated to local-hype to standardize naming.

Dockerfile (3)

32-32: Change approved: Updated binary name and build flags.

The binary name and build flags have been correctly updated to reflect the new naming conventions.


36-36: Change approved: Updated binary name in COPY instruction.

The binary name has been correctly updated in the COPY instruction to reflect the new naming conventions.


38-38: Change approved: Updated ENTRYPOINT.

The ENTRYPOINT has been correctly updated to the new binary name to reflect the new naming conventions.

testnets/README.md (10)

1-1: Change approved: Updated testnet name.

The testnet name has been correctly updated to "Hyperia Testnet" to reflect the new naming conventions.


18-18: Change approved: Updated binary name in installation instructions.

The binary name has been correctly updated in the installation instructions to reflect the new naming conventions.


26-26: Change approved: Updated binary name in version check.

The binary name has been correctly updated in the version check instructions to reflect the new naming conventions.


29-29: Change approved: Updated binary name in long version check.

The binary name has been correctly updated in the long version check instructions to reflect the new naming conventions.


35-35: Change approved: Updated binary name and chain ID in chain initialization.

The binary name and chain ID have been correctly updated in the chain initialization instructions to reflect the new naming conventions.


41-41: Change approved: Updated path to genesis file.

The path to the genesis file has been correctly updated to reflect the new naming conventions.


49-49: Change approved: Updated binary name in wallet creation.

The binary name has been correctly updated in the wallet creation instructions to reflect the new naming conventions.


55-55: Change approved: Updated binary name and denomination in genesis account funding.

The binary name and denomination have been correctly updated in the genesis account funding instructions to reflect the new naming conventions.


61-61: Change approved: Updated binary name, denomination, and chain ID in gentx creation.

The binary name, denomination, and chain ID have been correctly updated in the gentx creation instructions to reflect the new naming conventions.


67-67: Change approved: Updated path to gentx file.

The path to the gentx file has been correctly updated to reflect the new naming conventions.

scripts/tests/alliance/delegate.sh (9)

9-9: Change approved: Updated binary name.

The binary name has been correctly updated to hyperiad to standardize naming.


13-13: Change approved: Updated denomination.

The denomination has been correctly updated to uhype to standardize naming.


18-18: Address existing comment: Accessing localhost.

Accessing localhost could indicate debug code or could hinder scaling. Do not leave debug code in production.

Tools
GitHub Check: devskim

[notice] 18-18: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production


32-32: Change approved: Updated denomination check.

The denomination check has been correctly updated to reflect the new naming conventions.


56-56: Change approved: Updated denomination in alliance creation.

The denomination has been correctly updated in the alliance creation instructions to reflect the new naming conventions.


64-64: Address existing comment: Accessing localhost.

Accessing localhost could indicate debug code or could hinder scaling. Do not leave debug code in production.

Tools
GitHub Check: devskim

[notice] 64-64: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production


71-71: Address existing comment: Accessing localhost.

Accessing localhost could indicate debug code or could hinder scaling. Do not leave debug code in production.

Tools
GitHub Check: devskim

[notice] 71-71: Accessing localhost could indicate debug code, or could hinder scaling.
Do not leave debug code in production


95-96: Change approved: Updated denomination in total supply query.

The denomination has been correctly updated in the total supply query to reflect the new naming conventions.


101-101: Change approved: Updated denomination in error message.

The denomination has been correctly updated in the error message to reflect the new naming conventions.

scripts/tests/init-test-framework.sh (3)

3-9: LGTM! Consistent variable updates.

The updates to the binary name, chain IDs, and denomination are consistent with the PR objectives.


91-92: LGTM! Correct updates to gentx transactions.

The updates to the gentx transactions to use the new denomination uhype are correct.


Line range hint 129-136:
LGTM! Correct updates to genesis file parameters.

The updates to the genesis file parameters to use the new denomination uhype are correctly applied.

@@ -83,7 +83,7 @@

echo "Delegating $AMOUNT_TO_DELEGATE to the alliance $IBC_DENOM"
VAL_ADDR=$($BINARY query staking validators --output json | jq .validators[0].operator_address --raw-output)
DELEGATE_RES=$($BINARY tx alliance delegate $VAL_ADDR $AMOUNT_TO_DELEGATE$IBC_DENOM --from=node0 --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test --fees 600000$UWHALE_DENOM --gas 500000 --chain-id=test-2 -o json -y)
DELEGATE_RES=$($BINARY tx alliance delegate $VAL_ADDR $AMOUNT_TO_DELEGATE$IBC_DENOM --from=node0 --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test --fees 600000$UHYPE_DENOM --gas 500000 --chain-id=test-2 -o json -y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused variable.

The DELEGATE_RES variable is declared but not used in the script. Consider removing it to avoid confusion.

- DELEGATE_RES=$($BINARY tx alliance delegate $VAL_ADDR $AMOUNT_TO_DELEGATE$IBC_DENOM --from=node0 --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test  --fees 600000$UHYPE_DENOM --gas 500000 --chain-id=test-2 -o json -y)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
DELEGATE_RES=$($BINARY tx alliance delegate $VAL_ADDR $AMOUNT_TO_DELEGATE$IBC_DENOM --from=node0 --from=$VAL_WALLET_2 --home $CHAIN_DIR/test-2 --keyring-backend=test --fees 600000$UHYPE_DENOM --gas 500000 --chain-id=test-2 -o json -y)
Tools
Shellcheck

[warning] 86-86: DELEGATE_RES appears unused. Verify use (or export if used externally).

(SC2034)

@faddat
Copy link
Contributor

faddat commented Jul 8, 2024

Did you resolve the issue with Genesis?

@elix1er
Copy link
Contributor Author

elix1er commented Jul 8, 2024

ill hop on it again quick, maybe some plain default genesis file does the job

@elix1er
Copy link
Contributor Author

elix1er commented Jul 8, 2024

w8 i got a hint what it could be

@elix1er
Copy link
Contributor Author

elix1er commented Jul 8, 2024

i mean, the node starts running now, patched the function in app.go

func (app *HyperiaApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.ResponseFinalizeBlock, error) {

to not fire a panic anymore - but firing following errors:

4:27AM INF Attempting to migrate consensus parameters error="rpc error: code = Internal desc = collections: not found: key 'no_key' of type github.com/cosmos/gogoproto/tendermint.types.ConsensusParams" module=server
4:27AM ERR baseAppLegacySS is invalid module=server

... not sure if that is a good or bad sign

Screenshot 2024-07-09 at 04 29 34

ref: cosmos/cosmos-sdk#20160

err := baseapp.MigrateParams(sdk.UnwrapSDKContext(ctx), baseAppLegacySS, app.ConsensusParamsKeeper.ParamsStore)
if err != nil {
panic(err)
app.Logger().Error("Failed to migrate params", "error", err)
// TODO: Consider how to handle this error without panicking

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
scripts/wasm/gov_upload_code.sh (2)

2-2: Consider removing or using the VOTER variable.

The VOTER variable is declared but not used in the script. If it's not needed, consider removing it to clean up the code.

- VOTER=

7-7: Consider removing or using the VAL variable.

The VAL variable is declared but not used in the script. If it's not needed, consider removing it to clean up the code.

- VAL=$($BINARY keys show -a $VAL_KEY --keyring-backend test)
Tools
Shellcheck

[warning] 7-7: VAL appears unused. Verify use (or export if used externally).

(SC2034)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9410b57 and 7b4a90b.

Files selected for processing (6)
  • app/ante/ante_test.go (3 hunks)
  • app/ante/suite.go (1 hunks)
  • app/app.go (2 hunks)
  • scripts/run-node.sh (4 hunks)
  • scripts/tests/init-test-framework.sh (4 hunks)
  • scripts/wasm/gov_upload_code.sh (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • scripts/tests/init-test-framework.sh
Additional context used
Shellcheck
scripts/wasm/gov_upload_code.sh

[warning] 6-6: VOTER appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 7-7: VAL appears unused. Verify use (or export if used externally).

(SC2034)

GitHub Check: devskim
app/app.go

[notice] 1096-1096: A "TODO" or similar was left in source code, possibly indicating incomplete functionality
Suspicious comment

Additional comments not posted (14)
scripts/wasm/gov_upload_code.sh (2)

10-10: LGTM!

The changes to update the denomination to uhype are correct.


20-20: LGTM!

The changes to update the denomination to uhype are correct.

scripts/run-node.sh (5)

14-14: Ensure the pkill command targets the correct process.

The pkill command is used to kill the hyperiad process. Ensure that this is the correct process to target.


17-17: LGTM!

The changes to update the denomination to uhype are correct.


35-38: LGTM!

The changes to update the binary name to hyperiad are correct.


41-41: LGTM!

The changes to update the chain ID to test-hype-1 are correct.


56-64: LGTM!

The changes to update the genesis configurations to use uhype are correct.

app/ante/ante_test.go (5)

21-21: LGTM!

The changes to update the denomination to uhype are correct.


23-23: LGTM!

The changes to update the denomination to uhype are correct.


58-58: LGTM!

The changes to update the denomination to uhype are correct.


69-69: LGTM!

The changes to update the denomination to uhype are correct.


77-77: LGTM!

The changes to update the denomination to uhype are correct.

app/ante/suite.go (1)

117-117: LGTM!

The changes to update the denomination to uhype are correct.

app/app.go (1)

1071-1099: Improve error handling and logging.

The changes improve error handling and logging for consensus parameter migration. Ensure that the error handling strategy aligns with the overall application architecture.

Tools
GitHub Check: devskim

[notice] 1096-1096: A "TODO" or similar was left in source code, possibly indicating incomplete functionality
Suspicious comment

app/app.go Show resolved Hide resolved
$SED_BINARY -i 's/minimum-gas-prices = "0.25ueve"/minimum-gas-prices = "0.0eve"/' $HOME_DIR/config/app.toml


$SED_BINARY -i 's/minimum-gas-prices = "0.25uhype"/minimum-gas-prices = "0.0hype"/' $HOME_DIR/config/app.toml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the minimum gas price denomination.

The minimum gas price should be set to 0.0uhype instead of 0.0hype.

- $SED_BINARY -i 's/minimum-gas-prices = "0.25uhype"/minimum-gas-prices = "0.0hype"/' $HOME_DIR/config/app.toml
+ $SED_BINARY -i 's/minimum-gas-prices = "0.25uhype"/minimum-gas-prices = "0.0uhype"/' $HOME_DIR/config/app.toml
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$SED_BINARY -i 's/minimum-gas-prices = "0.25uhype"/minimum-gas-prices = "0.0hype"/' $HOME_DIR/config/app.toml
$SED_BINARY -i 's/minimum-gas-prices = "0.25uhype"/minimum-gas-prices = "0.0uhype"/' $HOME_DIR/config/app.toml

@elix1er
Copy link
Contributor Author

elix1er commented Jul 9, 2024

i mean, check this:

https://pkg.go.dev/github.com/cosmos/cosmos-sdk/baseapp#pkg-overview

Deprecated.

Legacy types are defined below to aid in the migration of CometBFT consensus parameters from use of the now deprecated x/params modules to a new dedicated x/consensus module.

Application developers should ensure that they implement their upgrade handler correctly such that app.ConsensusParamsKeeper.Set() is called with the values returned by GetConsensusParams().

Example:

baseAppLegacySS := app.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable())

app.UpgradeKeeper.SetUpgradeHandler(
	UpgradeName,
	func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
		if cp := baseapp.GetConsensusParams(ctx, baseAppLegacySS); cp != nil {
			app.ConsensusParamsKeeper.Set(ctx, cp)
		} else {
			ctx.Logger().Info("warning: consensus parameters are undefined; skipping migration", "upgrade", UpgradeName)
		}

		return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM)
	},
)

Developers can also bypass the use of the legacy Params subspace and set the values to app.ConsensusParamsKeeper.Set() explicitly.

Note, for new chains this is not necessary as CometBFT's consensus parameters will automatically be set for you in InitChain.

Node Log:

{"level":"info","module":"server","error":"rpc error: code = Internal desc = collections: not found: key 'no_key' of type github.com/cosmos/gogoproto/tendermint.types.ConsensusParams","time":"2024-07-09T08:53:41+07:00","message":"Attempting to migrate consensus parameters"}
{"level":"error","module":"server","time":"2024-07-09T08:53:41+07:00","message":"baseAppLegacySS is invalid"}

@elix1er
Copy link
Contributor Author

elix1er commented Jul 9, 2024

great, wasm test passing - but does anyone know a fix for this annoying logspam with
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation ?

❯ ./scripts/wasm/test_wasm.sh
Starting chain...
Wasm file already exists for cw20_base
Uploading contract...
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
gas estimate: 2784050
code: 0
codespace: ""
data: ""
events: []
gas_used: "0"
gas_wanted: "0"
height: "0"
info: ""
logs: []
raw_log: ""
timestamp: ""
tx: null
txhash: C6E838ED45BAAE1F571FBAF376C4973E51AF105848E54BE2D3404C9CFEBD7121
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
code_infos: []
pagination:
  next_key: null
  total: "0"
Instantiating contract...
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation

- address: hype12lzflhjry5mkh03x5200mkwqgk32lxutqpmem7
  name: demo
  pubkey: '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"AmCiqZBD3JI/ehN00k8WJZTToK0zzfDmfG1ASynA7+rW"}'
  type: local


**Important** write this mnemonic phrase in a safe place.
It is the only way to recover your account if you ever forget your password.

betray plunge alert aspect stomach bicycle road believe write goddess inherit legal chaos image merit point able suffer combine expire hospital chronic title left
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
gas estimate: 274111
code: 0
codespace: ""
data: ""
events: []
gas_used: "0"
gas_wanted: "0"
height: "0"
info: ""
logs: []
raw_log: ""
timestamp: ""
tx: null
txhash: C4FB22F606573D97AB68883CA105013C89B2B610741F6E6BCF34A99C7A2EE19C
Waiting for tx to be processed...
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
Validator Balance:
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
data:
  balance: "123456789000"
Demo Balance:
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
data:
  balance: "0"
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
gas estimate: 215246
code: 0
codespace: ""
data: ""
events: []
gas_used: "0"
gas_wanted: "0"
height: "0"
info: ""
logs: []
raw_log: ""
timestamp: ""
tx: null
txhash: 8D0B84A6B8729B9189CCABE25463CAF69152F0DD72B92DAF6F80978BF05259C4
Waiting for tx to be processed...
Validator Balance:
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
data:
  balance: "122469134679"
Demo Balance:
service alliance.alliance.Msg does not have cosmos.msg.v1.service proto annotation
data:
  balance: "987654321"
Success executing basic wasm tests!
~/Documents/GitHub/hyperia feat--hyperiad* 1m 11s                                                                                                                                                         09:48:44 AM
❯ 

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7b4a90b and ac78665.

Files selected for processing (5)
  • scripts/run-node.sh (4 hunks)
  • scripts/tests/init-test-framework.sh (5 hunks)
  • scripts/wasm/instantiate_cw20.sh (3 hunks)
  • scripts/wasm/test_wasm.sh (1 hunks)
  • scripts/wasm/upload_code.sh (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • scripts/tests/init-test-framework.sh
  • scripts/wasm/test_wasm.sh
Additional context used
Shellcheck
scripts/wasm/upload_code.sh

[warning] 9-9: DENOM appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 13-13: TEST0_ADDRESS appears unused. Verify use (or export if used externally).

(SC2034)

Additional comments not posted (9)
scripts/wasm/upload_code.sh (2)

3-3: LGTM!

The BINARY variable is correctly updated to build/hyperiad.


5-5: LGTM!

The CHAIN_ID variable is correctly updated to local-hype.

scripts/wasm/instantiate_cw20.sh (2)

5-5: LGTM!

The CHAIN_ID variable is correctly updated to local-hype.


6-6: LGTM!

The BINARY variable is correctly updated to build/hyperiad.

scripts/run-node.sh (5)

14-14: LGTM!

The pkill command is correctly updated to pkill -9 hyperiad.


17-17: LGTM!

The DENOM variable is correctly updated to uhype.


38-38: LGTM!

The BINARY variable is correctly updated to build/hyperiad.


41-41: LGTM!

The CHAIN_ID variable is correctly updated to local-hype.


72-72: LGTM!

The minimum gas price denomination is correctly updated to uhype.


CONTRACT=cw20_base
HOME=mytestnet
DENOM="ueve"
DENOM="uhype"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused variable.

The DENOM variable is declared but not used in the script. Consider removing it to avoid confusion.

- DENOM="uhype"
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
DENOM="uhype"
Tools
Shellcheck

[warning] 9-9: DENOM appears unused. Verify use (or export if used externally).

(SC2034)

@elix1er
Copy link
Contributor Author

elix1er commented Jul 13, 2024

probably gonna redo this PR in smaller steps, rather than as it's current state, fyi

@faddat
Copy link
Contributor

faddat commented Jul 30, 2024

Hey, actually this is pretty good and thank you. I'm going to live through it I'll probably push to your door but I'll be using your commits. thank you so much

@faddat faddat merged commit 18e0359 into HyperiaNetwork:main Aug 20, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants