imp(tests): Add Solidity and Ledger tests #41
Conversation
MalteHerrmann
requested review from
0xstepit and
GAtom22
and removed request for
a team
WalkthroughThe changes involve the addition and enhancement of testing capabilities across the codebase, including the introduction of Solidity and Ledger tests, a new Bash script for running Solidity tests, and various mock implementations for integration testing. Furthermore, configuration files for Truffle and Hardhat have been established, ensuring a consistent environment for deploying and managing smart contracts on the Evmos network. Documentation has also been improved to support these updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Local_Node
participant Solidity_Tests
participant Ledger_Tests
participant Evmos_Network
Developer->>Local_Node: Start Node with Trace
Local_Node-->>Developer: Node Running
Developer->>Solidity_Tests: Run Solidity Tests
Solidity_Tests->>Evmos_Network: Deploy Contracts
Evmos_Network-->>Solidity_Tests: Contract Deployed
Solidity_Tests-->>Developer: Test Results
Developer->>Ledger_Tests: Run Ledger Tests
Ledger_Tests->>Evmos_Network: Interact with Ledger
Evmos_Network-->>Ledger_Tests: Ledger Interaction Results
Ledger_Tests-->>Developer: Test Results
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 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: 8
Outside diff range, codebase verification and nitpick comments (18)
tests/solidity/suites/eip1559/truffle-config.js (1)
1-1: Consider removing unused imports.The
HDWalletProviderimport is commented out and not used in the file.Remove the commented import if it's not needed to keep the file clean.
tests/solidity/suites/opcode/contracts/Migrations.sol (1)
11-13: Enhance modifier documentation.Consider adding comments to explain the purpose of the
restrictedmodifier.This will improve code readability and maintainability.
tests/solidity/suites/basic/contracts/Counter.sol (2)
6-6: Consider initializing the counter in the constructor.The counter is initialized to
0by default. Consider explicitly setting it in the constructor for clarity.This can improve code readability.
7-7: Clarify error message constant.The
ERROR_TOO_LOWconstant is used in thesubtractfunction. Consider adding a comment to explain its purpose.This will improve code readability and maintainability.
tests/solidity/suites/precompiles/test/staking.js (2)
5-25: Consider adding error handling for contract interactions.While the test checks the staking functionality, it would be beneficial to add error handling for the contract interactions to provide more informative test failures.
You can wrap the contract interaction in a try-catch block and assert that no errors are thrown.
5-25: Enhance test description for clarity.The test description "should stake EVMOS to a validator" could be more descriptive to clarify the expected behavior and conditions.
Consider expanding the description to include details about the initial setup and expected outcome.
tests/solidity/suites/basic/test/revert.js (2)
5-18: Improve error message handling inexpectRevert.The utility function
expectRevertchecks for a revert error, but the error message handling could be more robust by checking for specific revert reasons if applicable.Consider enhancing the error message check to include specific revert reasons, if they are expected.
28-30: Add assertions to verify contract state after revert.The test case checks for a revert, but it would be beneficial to include assertions that verify the contract state remains unchanged after the revert.
Add assertions to ensure the contract state is as expected after the revert.
tests/solidity/suites/opcode/test/opCodes.js (3)
11-20: Add more detailed assertions for opcode execution.The test case checks for the absence of errors during opcode execution, but additional assertions could verify specific expected outcomes.
Consider adding assertions to verify the contract's state or return values after opcode execution.
22-30: Improve error handling for invalid opcode test.The test case checks for an error when an invalid opcode is executed, but the error handling could be more informative by checking the error message content.
Enhance the error handling to assert specific error messages or codes related to invalid opcodes.
32-40: Add assertions to verify contract state after revert.The test case checks for a revert, but it would be beneficial to include assertions that verify the contract state remains unchanged after the revert.
Add assertions to ensure the contract state is as expected after the revert.
tests/integration/ledger/mocks/registry.go (3)
14-14: Fix typo in comment.The comment contains a typo: "registrtry" should be "registry".
// Methods registrtry for SECP256k1 +// Methods registry for SECP256k1
42-42: Fix typo in comment.The comment contains a typo: "registrtry" should be "registry".
// Methods registrtry for AccountRetriever +// Methods registry for AccountRetriever
49-49: Fix typo in comment.The comment contains a typo: "ypes" should be "types".
// original: EnsureExists(client.Context, ypes.AccAddress) error +// original: EnsureExists(client.Context, types.AccAddress) errortests/solidity/suites/basic/test/counter.js (2)
5-18: Improve error handling inexpectRevert.The
expectRevertfunction checks for a revert but could be improved by checking for specific error messages or codes if available.
28-61: Consider adding assertions for gas usage.In the
addtest, consider adding assertions to check gas usage for transactions to ensure efficiency.tests/solidity/suites/opcode/contracts/OpCodes.sol (1)
10-300: Consider adding comments for clarity in assembly sections.The contract
OpCodesis comprehensive, but additional comments explaining the purpose of each assembly block would improve readability.example_chain/local_node.sh (1)
202-202: Verify the definition of the"$TRACE"variable inexample_chain/local_node.sh.The
"$TRACE"variable is used in theosd startcommand inexample_chain/local_node.shbut is not defined within this file. Ensure that it is correctly defined or imported before use to avoid potential runtime issues.
example_chain/local_node.sh: Ensure"$TRACE"is defined or imported.Analysis chain
Verify the definition and usage of the
"$TRACE"variable.The addition of
"$TRACE"to theosd startcommand suggests enabling tracing. Ensure that the"$TRACE"variable is correctly defined and used within the script.Run the following script to verify the definition and usage of the
"$TRACE"variable:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of the `"$TRACE"` variable. # Test: Search for the `TRACE` variable definition and usage. Expect: At least one definition and usage. rg --type shell 'TRACE'Length of output: 53
Script:
#!/bin/bash # Description: Verify the definition and usage of the `"$TRACE"` variable. # Test: Search for the `TRACE` variable definition and usage across all files. rg 'TRACE'Length of output: 190
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
tests/solidity/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (42)
- CHANGELOG.md (1 hunks)
- example_chain/local_node.sh (1 hunks)
- scripts/run-solidity-tests.sh (1 hunks)
- tests/integration/ledger/evmosd_suite_test.go (1 hunks)
- tests/integration/ledger/ledger_test.go (1 hunks)
- tests/integration/ledger/mocks/AccountRetriever.go (1 hunks)
- tests/integration/ledger/mocks/SECP256K1.go (1 hunks)
- tests/integration/ledger/mocks/registry.go (1 hunks)
- tests/integration/ledger/mocks/tendermint.go (1 hunks)
- tests/solidity/.gitattributes (1 hunks)
- tests/solidity/.gitignore (1 hunks)
- tests/solidity/init-node.sh (1 hunks)
- tests/solidity/package.json (1 hunks)
- tests/solidity/suites/basic/contracts/Counter.sol (1 hunks)
- tests/solidity/suites/basic/contracts/EventTest.sol (1 hunks)
- tests/solidity/suites/basic/contracts/Storage.sol (1 hunks)
- tests/solidity/suites/basic/package.json (1 hunks)
- tests/solidity/suites/basic/test/counter.js (1 hunks)
- tests/solidity/suites/basic/test/events.js (1 hunks)
- tests/solidity/suites/basic/test/revert.js (1 hunks)
- tests/solidity/suites/basic/test/storage.js (1 hunks)
- tests/solidity/suites/basic/truffle-config.js (1 hunks)
- tests/solidity/suites/eip1559/package.json (1 hunks)
- tests/solidity/suites/eip1559/test/eip1559.js (1 hunks)
- tests/solidity/suites/eip1559/truffle-config.js (1 hunks)
- tests/solidity/suites/exception/contracts/TestRevert.sol (1 hunks)
- tests/solidity/suites/exception/contracts/test/Migrations.sol (1 hunks)
- tests/solidity/suites/exception/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/exception/package.json (1 hunks)
- tests/solidity/suites/exception/test/revert.js (1 hunks)
- tests/solidity/suites/exception/truffle-config.js (1 hunks)
- tests/solidity/suites/opcode/contracts/Migrations.sol (1 hunks)
- tests/solidity/suites/opcode/contracts/OpCodes.sol (1 hunks)
- tests/solidity/suites/opcode/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js (1 hunks)
- tests/solidity/suites/opcode/package.json (1 hunks)
- tests/solidity/suites/opcode/test/opCodes.js (1 hunks)
- tests/solidity/suites/opcode/truffle-config.js (1 hunks)
- tests/solidity/suites/precompiles/hardhat.config.js (1 hunks)
- tests/solidity/suites/precompiles/package.json (1 hunks)
- tests/solidity/suites/precompiles/test/staking.js (1 hunks)
- tests/solidity/test-helper.js (1 hunks)
Files skipped from review due to trivial changes (11)
- CHANGELOG.md
- tests/integration/ledger/mocks/SECP256K1.go
- tests/solidity/.gitattributes
- tests/solidity/.gitignore
- tests/solidity/suites/basic/package.json
- tests/solidity/suites/eip1559/package.json
- tests/solidity/suites/exception/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js
- tests/solidity/suites/opcode/package.json
- tests/solidity/suites/opcode/truffle-config.js
Additional comments not posted (66)
tests/solidity/suites/exception/contracts/test/Migrations.sol (1)
1-19: Standard Migration Contract.This is a standard migration contract used in Truffle projects. The code is correct and follows Solidity best practices.
tests/solidity/suites/basic/truffle-config.js (1)
1-17: Truffle Configuration for Local Development.The configuration is appropriate for a local development environment with reasonable gas settings and a specified Solidity compiler version.
tests/solidity/suites/exception/truffle-config.js (1)
1-17: Truffle Configuration for Local Development.The configuration is consistent with the
basicsuite and is suitable for local development with appropriate settings.tests/solidity/suites/exception/package.json (1)
1-26: Configuration looks good.The
package.jsonis well-structured with appropriate scripts and dependencies for testing.tests/solidity/suites/eip1559/truffle-config.js (2)
6-11: Verify network configuration settings.Ensure that the
evmosnetwork settings are correct for your local development environment.Double-check that the host, port, gas, and gas price align with your testing requirements.
16-16: Specify a stable compiler version.The Solidity compiler version is set to
0.8.18. Ensure this version is compatible with all contracts.Consider locking the version to a specific patch version if stability is a concern.
tests/solidity/suites/basic/contracts/Counter.sol (1)
1-1: Ensure license compliance.The SPDX license identifier is set to GPL-3.0. Verify that this license is appropriate for your project.
Ensure that all dependencies and project requirements are compatible with this license.
scripts/run-solidity-tests.sh (4)
1-3: Environment setup looks good.The script correctly sets up the GOPATH and updates the PATH.
5-6: Data removal command is safe.The script safely removes a temporary directory used for Solidity tests.
8-9: Error handling setup is appropriate.Using
set -eensures the script exits on the first error, which is a good practice for build scripts.
25-25: Test execution command is correct.The script correctly executes yarn tests with the specified network.
tests/solidity/suites/basic/contracts/Storage.sol (4)
1-11: State variable declaration is correct.The contract correctly declares a state variable
numberfor storage.
13-19:storefunction implementation is correct.The function correctly assigns the input value to the state variable
number.
21-27:retrievefunction implementation is correct.The function correctly returns the value of the state variable
number.
29-31:shouldRevertfunction implementation is correct.The function is designed to always revert, which is correctly implemented using
require(false).tests/solidity/suites/exception/test/revert.js (2)
1-10: Test setup and contract instantiation are correct.The test file correctly sets up a new contract instance before each test.
11-23: Test case for revert behavior is correct.The test case correctly verifies the revert behavior and expected state changes.
tests/solidity/package.json (5)
1-6: Package metadata looks good.The package metadata is correctly defined and follows standard conventions.
7-14: Workspaces configuration is appropriate.The workspaces and nohoist settings are correctly defined for managing monorepo setups.
15-19: Dependencies are well-defined.The dependencies are relevant and versioned, which is a good practice for maintaining consistency.
20-23: Scripts are correctly set up.The test and postinstall scripts are appropriate for the package's functionality.
24-36: Standard configuration is suitable.The global variables are correctly defined for the testing environment.
tests/solidity/suites/basic/contracts/EventTest.sol (3)
1-11: Contract metadata and state variable are correctly defined.The license, pragma directive, and state variable are appropriate and follow Solidity conventions.
13-24: Events are well-defined.The events are appropriately defined and indexed, providing useful information for state changes.
26-35: Functions are correctly implemented.The functions for storing values and emitting events are well-implemented and provide transparency for state changes.
tests/solidity/suites/basic/test/storage.js (3)
5-12: Deployment test is correctly implemented.The test ensures the Storage contract is deployed with a valid address, which is essential for further testing.
14-18: Store value test is correctly implemented.The test verifies that a value is successfully stored in the contract, ensuring the transaction is successful.
20-23: Retrieve value test is correctly implemented.The test ensures that the stored value can be accurately retrieved from the contract.
tests/solidity/suites/exception/contracts/TestRevert.sol (2)
4-15: LGTM!The
Statecontract is well-implemented with clear functionality for setting and querying the state variable.
18-43: LGTM!The
TestRevertcontract effectively demonstrates exception handling with the try-catch block and provides clear functions for querying state.tests/solidity/suites/basic/test/events.js (2)
9-13: LGTM!The test case for deploying the
EventTestcontract is correctly implemented and checks for a valid contract address.
15-31: LGTM!The test case for event emissions is well-implemented, using truffle-assertions to verify the correct events and values.
tests/integration/ledger/mocks/tendermint.go (3)
23-27: LGTM!The
NewMockTendermintRPCfunction correctly initializes the mock with the providedabci.ResponseQuery.
29-31: LGTM!The
BroadcastTxSyncmethod correctly simulates a successful transaction broadcast with code 0.
33-39: LGTM!The
ABCIQueryWithOptionsmethod correctly returns the mock response query, simulating an ABCI query.tests/solidity/suites/precompiles/package.json (2)
6-10: Ensure contract paths are correct and efficient.The
get-contractsscript usesrsyncto synchronize Solidity contracts. Verify that the paths are correct and efficient for your project's structure.
12-34: Review dependency versions for compatibility.Ensure that the specified versions of dependencies are compatible with each other and with the project's requirements. This is especially important for packages like
hardhat,ethers, andopenzeppelinwhich are frequently updated.tests/solidity/suites/basic/test/counter.js (1)
20-26: Ensure consistent use ofbeforeEach.The
beforeEachhook initializes theCountercontract. Ensure this setup is consistent with other test files to maintain uniformity.tests/integration/ledger/mocks/AccountRetriever.go (5)
17-29: LGTM: Mock functionEnsureExistsis correctly implemented.The function uses the
Calledmethod to simulate the function call and retrieve the expected return value.
31-52: LGTM: Mock functionGetAccountis correctly implemented.The function handles return values using the
Calledmethod and checks for nil values.
54-80: LGTM: Mock functionGetAccountNumberSequenceis correctly implemented.The function uses the
Calledmethod to simulate the function call and retrieve the expected return values.
82-110: LGTM: Mock functionGetAccountWithHeightis correctly implemented.The function uses the
Calledmethod to simulate the function call and retrieve the expected return values.
117-125: LGTM: Mock constructorNewAccountRetrieveris correctly implemented.The function sets up the mock and registers a cleanup function to assert expectations.
tests/integration/ledger/evmosd_suite_test.go (6)
59-65: LGTM: Test functionTestLedgeris correctly implemented.The function initializes the
LedgerTestSuiteand runs it using thesuite.Runmethod.
67-81: LGTM: Setup functionSetupTestis correctly implemented.The function sets up the ledger mock and initializes the private and public keys for the test suite.
83-113: LGTM: Setup functionSetupEvmosAppis correctly implemented.The function initializes the Evmos application and sets up the context with a new block header.
115-144: LGTM: FunctionNewKeyringAndCtxsis correctly implemented.The function creates a new keyring and client context for testing purposes, setting up the necessary components.
146-167: LGTM: FunctionevmosAddKeyCmdis correctly implemented.The function creates a new Cobra command for adding a key from the ledger, setting up the command with appropriate flags and a run function.
169-177: LGTM: FunctionMockKeyringOptionis correctly implemented.The function returns a keyring option that configures the mock ledger with necessary options.
tests/integration/ledger/ledger_test.go (5)
56-99: LGTM: Test case "Adding a key from ledger using the CLI" is well-defined.The test case verifies the functionality of adding a key from the ledger using the CLI with different algorithms and handles errors appropriately.
101-165: LGTM: Test case "Signing a transaction" is well-defined.The test case verifies the functionality of signing transactions with the ledger key and checks for valid signatures and error handling.
130-166: LGTM: Test case "Perform bank send with keyring functions" is well-defined.The test case verifies the functionality of performing a bank send transaction using keyring functions and checks for valid signatures and error handling.
167-211: LGTM: Test case "Perform bank send with CLI command" is well-defined.The test case verifies the functionality of performing a bank send transaction using the CLI command and checks for successful execution and error handling.
212-230: LGTM: Test case "Error handling for ledger device" is well-defined.The test case verifies the error handling when the ledger device returns an error during transaction execution and checks for proper error propagation and comparison.
tests/solidity/test-helper.js (6)
7-11: Logger object is well-implemented.The logger object provides clear and concise logging functionality.
88-149: Test loading logic is well-implemented.The function effectively loads and validates test suites with appropriate error handling.
152-170: Test suite execution logic is well-implemented.The function correctly manages the execution of test suites and handles process output.
173-187: Test execution logic is well-implemented.The function effectively manages the execution of multiple test suites.
189-240: Network setup logic is well-implemented.The function effectively manages network setup with appropriate logging and timeout handling.
242-264: Main function is well-structured.The function orchestrates the test setup and execution effectively.
tests/solidity/init-node.sh (5)
1-12: Initial setup is clear, but note the TODOs.The initial variable setup is well-documented, and the keyring warning is a good security practice. Address the TODO comments for further improvements.
22-26: Dependency validation is well-implemented.The script correctly checks for the
jqdependency and provides a helpful error message if not found.
58-110: Key import and genesis setup are well-implemented.The script effectively manages key import from mnemonics and configures the genesis file using
jq.
126-136: Genesis transaction signing and API setup are well-implemented.The script correctly signs the genesis transaction and enables necessary APIs for testing.
147-153: Node start command is well-implemented.The script effectively starts the node with appropriate configurations for testing.
tests/solidity/suites/opcode/contracts/OpCodes.sol (1)
3-8: ContractTest1is well-implemented.The function
isSameAddresscorrectly compares two addresses.
There was a problem hiding this comment.
Actionable comments posted: 16
Outside diff range, codebase verification and nitpick comments (11)
tests/solidity/suites/opcode/contracts/Migrations.sol (1)
1-1: Consider updating the Solidity pragma version.The current pragma version
>=0.4.21 <0.6.0is outdated. Consider updating it to a more recent version to leverage the latest features and security improvements.tests/solidity/suites/basic/contracts/Counter.sol (2)
3-3: Consider updating the Solidity pragma version.The current pragma version
>=0.7.0 <0.9.0is slightly outdated. Consider updating it to a more recent version to leverage the latest features and security improvements.
6-6: Consider initializing the counter in the constructor.While the counter is initialized to zero by default, explicitly initializing it in the constructor can improve code clarity.
scripts/run-solidity-tests.sh (1)
2-3: Consider checking if GOPATH is already set.Before setting
GOPATH, check if it is already set to avoid overwriting existing configurations.tests/solidity/suites/basic/contracts/Storage.sol (2)
9-11: Consider initializing state variables.While not strictly necessary, initializing state variables can help avoid potential issues with uninitialized storage.
29-31: Clarify the purpose ofshouldRevert.The
shouldRevertfunction is designed to always revert. Ensure this is documented in the comments to clarify its purpose for future developers.tests/solidity/package.json (1)
20-23: Consider adding more descriptive script names.The script names like
testandpostinstallcould be more descriptive to convey their purpose clearly, especially as the project grows.tests/solidity/suites/basic/contracts/EventTest.sol (1)
9-37: Consider adding a function to retrieve the stored value.The contract currently allows storing values and emitting events but lacks a function to retrieve the stored value. Adding a getter function can enhance the contract's usability.
Consider adding a function like this:
function retrieve() public view returns (uint256) { return number; }tests/solidity/suites/precompiles/test/staking.js (1)
5-26: Consider adding error handling for the staking transaction.The test currently assumes that the staking transaction will succeed. Adding error handling can improve test robustness by catching unexpected failures.
Consider wrapping the staking transaction in a try-catch block:
try { const tx = await staking.connect(signer).delegate(signer, valAddr, stakeAmount) await tx.wait(1) } catch (error) { console.error('Staking transaction failed:', error) }tests/solidity/suites/basic/test/revert.js (1)
5-18: Ensure proper error message handling inexpectRevert.The
expectRevertfunction checks for a revert error, but the error message comparison could be improved for clarity. Consider using a more specific error message check if possible.tests/solidity/suites/exception/contracts/TestRevert.sol (1)
5-9: Consider providing a revert reason inState.set.The
requirestatement in thesetfunction could include a revert reason to improve error traceability.Apply this diff to add a revert reason:
require(a < 10, "Input must be less than 10");
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
tests/solidity/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (42)
- CHANGELOG.md (1 hunks)
- example_chain/local_node.sh (1 hunks)
- scripts/run-solidity-tests.sh (1 hunks)
- tests/integration/ledger/evmosd_suite_test.go (1 hunks)
- tests/integration/ledger/ledger_test.go (1 hunks)
- tests/integration/ledger/mocks/AccountRetriever.go (1 hunks)
- tests/integration/ledger/mocks/SECP256K1.go (1 hunks)
- tests/integration/ledger/mocks/registry.go (1 hunks)
- tests/integration/ledger/mocks/tendermint.go (1 hunks)
- tests/solidity/.gitattributes (1 hunks)
- tests/solidity/.gitignore (1 hunks)
- tests/solidity/init-node.sh (1 hunks)
- tests/solidity/package.json (1 hunks)
- tests/solidity/suites/basic/contracts/Counter.sol (1 hunks)
- tests/solidity/suites/basic/contracts/EventTest.sol (1 hunks)
- tests/solidity/suites/basic/contracts/Storage.sol (1 hunks)
- tests/solidity/suites/basic/package.json (1 hunks)
- tests/solidity/suites/basic/test/counter.js (1 hunks)
- tests/solidity/suites/basic/test/events.js (1 hunks)
- tests/solidity/suites/basic/test/revert.js (1 hunks)
- tests/solidity/suites/basic/test/storage.js (1 hunks)
- tests/solidity/suites/basic/truffle-config.js (1 hunks)
- tests/solidity/suites/eip1559/package.json (1 hunks)
- tests/solidity/suites/eip1559/test/eip1559.js (1 hunks)
- tests/solidity/suites/eip1559/truffle-config.js (1 hunks)
- tests/solidity/suites/exception/contracts/TestRevert.sol (1 hunks)
- tests/solidity/suites/exception/contracts/test/Migrations.sol (1 hunks)
- tests/solidity/suites/exception/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/exception/package.json (1 hunks)
- tests/solidity/suites/exception/test/revert.js (1 hunks)
- tests/solidity/suites/exception/truffle-config.js (1 hunks)
- tests/solidity/suites/opcode/contracts/Migrations.sol (1 hunks)
- tests/solidity/suites/opcode/contracts/OpCodes.sol (1 hunks)
- tests/solidity/suites/opcode/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js (1 hunks)
- tests/solidity/suites/opcode/package.json (1 hunks)
- tests/solidity/suites/opcode/test/opCodes.js (1 hunks)
- tests/solidity/suites/opcode/truffle-config.js (1 hunks)
- tests/solidity/suites/precompiles/hardhat.config.js (1 hunks)
- tests/solidity/suites/precompiles/package.json (1 hunks)
- tests/solidity/suites/precompiles/test/staking.js (1 hunks)
- tests/solidity/test-helper.js (1 hunks)
Files skipped from review due to trivial changes (8)
- CHANGELOG.md
- tests/solidity/.gitattributes
- tests/solidity/.gitignore
- tests/solidity/suites/eip1559/package.json
- tests/solidity/suites/exception/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js
- tests/solidity/suites/opcode/package.json
Additional comments not posted (58)
tests/solidity/suites/exception/contracts/test/Migrations.sol (2)
1-2: Ensure SPDX License Identifier is Correct.The SPDX license identifier is set to MIT. Verify that this is the intended license for the project.
4-19: Code Structure and Security Looks Good.The contract is well-structured, and the use of the
restrictedmodifier ensures that only the owner can callsetCompleted. This is a good practice for access control.tests/solidity/suites/basic/truffle-config.js (2)
1-10: Network Configuration Seems Appropriate.The network configuration for
evmosis set up for local development with reasonable defaults for gas and gas price. Ensure these settings align with your testing requirements.
12-16: Compiler Version Compatibility.The Solidity compiler version is set to 0.8.18, which is compatible with the
Migrations.solcontract. Ensure that all contracts in the project are compatible with this version.tests/solidity/suites/exception/truffle-config.js (2)
1-10: Consistent Network Configuration.The network configuration for
evmosis consistent with other test suites, which is good for maintaining a uniform testing environment.
12-16: Ensure Compiler Version Consistency Across Contracts.The Solidity compiler version is set to 0.8.18. Verify that all contracts in this suite are compatible with this version.
tests/solidity/suites/opcode/truffle-config.js (2)
4-10: Verify network configuration details.The network configuration is set for a local Evmos network. Ensure that the host, port, gas, and gasPrice settings align with your development environment requirements.
12-15: Check Solidity compiler version compatibility.The Solidity compiler version is set to
0.5.17. Verify that this version is compatible with your smart contracts and any dependencies.tests/solidity/suites/precompiles/hardhat.config.js (2)
5-7: Verify Solidity compiler version.The Solidity compiler version is set to
0.8.18. Ensure that this version is suitable for your smart contracts and any dependencies.
9-15: Review network configuration and account management.The network configuration is set for a local Evmos network with specific accounts. Ensure these account private keys are intended for testing and do not expose any sensitive information.
tests/solidity/suites/eip1559/test/eip1559.js (1)
17-17: Ensure transaction type check is comprehensive.The assertion checks if the transaction type is
0x2. Ensure that other relevant properties of the transaction are also validated to improve test robustness.tests/solidity/suites/exception/package.json (1)
1-26: LGTM!The
package.jsonfile is well-structured for a test suite, with appropriate scripts and dependencies.tests/solidity/suites/eip1559/truffle-config.js (1)
1-19: LGTM!The
truffle-config.jsfile is appropriately configured for local testing with the Evmos network.tests/solidity/suites/basic/package.json (1)
1-26: LGTM!The
package.jsonfile is well-structured for a test suite, with appropriate scripts and dependencies.tests/solidity/suites/opcode/contracts/Migrations.sol (2)
11-13: Ensure proper access control with therestrictedmodifier.The
restrictedmodifier is correctly used to limit access to certain functions. Ensure that the owner address is set correctly during contract deployment to prevent unauthorized access.
19-22: Verify the safety of theupgradefunction.The
upgradefunction allows setting a new contract address. Ensure that the new contract is compatible and secure to prevent potential vulnerabilities.tests/solidity/suites/basic/contracts/Counter.sol (1)
17-21: Ensure proper error handling in thesubtractfunction.The
requirestatement ensures the counter is not negative. Verify that this logic aligns with the intended behavior and that the error message is clear and informative.scripts/run-solidity-tests.sh (2)
12-12: Verify the existence of theexample_chaindirectory.Ensure that the
example_chaindirectory exists and contains the necessary files for building the chain binary.
25-25: Ensure the correct network configuration for Yarn tests.Verify that the
evmosnetwork configuration is correct and aligns with the testing requirements.tests/solidity/suites/basic/contracts/Storage.sol (1)
1-3: Ensure the Solidity version is appropriate.The pragma directive specifies Solidity version
>=0.7.0 <0.9.0. Ensure that this version range is compatible with the rest of your codebase and any dependencies.tests/solidity/package.json (2)
16-19: Verify dependency versions.Ensure that the specified versions of
truffle,yargs, andpatch-packageare compatible with your project and other dependencies.
24-36: Review standard globals configuration.Ensure that all necessary globals are included in the
standardconfiguration to prevent linting errors.tests/solidity/suites/basic/test/storage.js (2)
3-3: Verify the existence of theStoragecontract.The test references a
Storagecontract, but the contract definition is not provided in the context. Ensure that theStoragecontract exists and is correctly linked to the test.Run the following script to verify the existence of the
Storagecontract:Verification successful
The
Storagecontract exists and is correctly referenced.The
Storagecontract is present in the codebase, confirming its existence and proper linkage to the test file.
- The
Storagecontract definition was found in the Solidity files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `Storage` contract. # Test: Search for the contract definition. Expect: At least one occurrence. rg --type solidity -A 5 $'contract Storage'Length of output: 77
Script:
#!/bin/bash # Find all Solidity files and search for the `Storage` contract definition. fd --extension sol --exec rg 'contract Storage' {}Length of output: 73
20-23: Ensure theretrievefunction is implemented in theStoragecontract.The test assumes the existence of a
retrievefunction in theStoragecontract. Verify that this function is implemented.Run the following script to verify the implementation of the
retrievefunction:Verification successful
The
retrievefunction is implemented in theStoragecontract. The function is defined asfunction retrieve() public view returns (uint256){, confirming its presence in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `retrieve` function in the `Storage` contract. # Test: Search for the function implementation. Expect: At least one occurrence. rg --type solidity -A 5 $'function retrieve'Length of output: 78
Script:
#!/bin/bash # Find all Solidity files and search for the `retrieve` function within them. fd --extension sol --exec rg 'function retrieve' {}Length of output: 110
tests/solidity/suites/basic/test/revert.js (2)
22-26: Test case for contract deployment looks good.The test case successfully checks that the
Storagecontract is deployed and has a valid address.
28-30: Test case for revert behavior looks good.The test case correctly uses the
expectReverthelper to ensure that theshouldRevertfunction call results in a revert.tests/solidity/suites/opcode/test/opCodes.js (3)
11-20: Test case for running opcodes looks good.The test case verifies that the majority of opcodes execute without errors, which is essential for ensuring contract functionality.
22-30: Test case for invalid opcode handling looks good.The test case correctly checks that an invalid opcode results in an error, which is crucial for robust error handling.
32-40: Test case for revert behavior looks good.The test case ensures that the
test_revertfunction call results in a revert, which is important for verifying expected contract behavior.tests/solidity/suites/exception/contracts/TestRevert.sol (1)
18-43: TheTestRevertcontract looks good.The contract correctly uses try-catch to handle exceptions and provides functions to query state variables.
tests/solidity/suites/basic/test/events.js (2)
9-13: Deployment test looks good.The test correctly verifies that the
EventTestcontract is deployed by checking that the address is not undefined.
15-31: Event emission test is well-implemented.The test appropriately uses
truffle-assertionsto verify the emission of multiple events with correct values.tests/integration/ledger/mocks/tendermint.go (3)
23-27: Constructor for MockTendermintRPC is correct.The function initializes the mock with the provided
ResponseQueryas expected.
29-31: BroadcastTxSync mock is correctly implemented.The function simulates a successful transaction broadcast by returning a result with a code of 0.
33-39: ABCIQueryWithOptions mock is correctly implemented.The function returns the mock's
responseQuery, simulating an ABCI query.tests/solidity/suites/precompiles/package.json (3)
1-11: Metadata and scripts are well-defined.The metadata is appropriate, and the scripts cover essential tasks for contract management and testing.
12-31: DevDependencies are appropriate and up-to-date.The listed packages are relevant for the project's testing and development needs.
32-34: Dependencies are appropriate.The inclusion of
ethersis suitable for interacting with Ethereum networks.tests/integration/ledger/mocks/registry.go (7)
16-19: LGTM: Mock setup forClose.The mock setup for the
Closemethod is correctly implemented.
21-25: LGTM: Mock setup forGetPublicKeySECP256K1.The mock setup for the
GetPublicKeySECP256K1method is correctly implemented.
27-34: LGTM: Mock setup forGetAddressPubKeySECP256K1.The mock setup for the
GetAddressPubKeySECP256K1method is correctly implemented.
36-39: LGTM: Mock setup forSignSECP256K1.The mock setup for the
SignSECP256K1method is correctly implemented.
44-47: LGTM: Mock setup forGetAccount.The mock setup for the
GetAccountmethod is correctly implemented.
49-52: LGTM: Mock setup forEnsureExists.The mock setup for the
EnsureExistsmethod is correctly implemented.
54-57: LGTM: Mock setup forGetAccountNumberSequence.The mock setup for the
GetAccountNumberSequencemethod is correctly implemented.tests/integration/ledger/mocks/SECP256K1.go (5)
18-30: LGTM: Mock implementation forClose.The mock implementation for the
Closemethod is correctly implemented.
32-60: LGTM: Mock implementation forGetAddressPubKeySECP256K1.The mock implementation for the
GetAddressPubKeySECP256K1method is correctly implemented.
62-83: LGTM: Mock implementation forGetPublicKeySECP256K1.The mock implementation for the
GetPublicKeySECP256K1method is correctly implemented.
85-94: LGTM: Mock implementation forSignSECP256K1.The mock implementation for the
SignSECP256K1method is correctly implemented.
101-108: LGTM: Constructor forSECP256K1.The constructor for the
SECP256K1mock is correctly implemented.tests/solidity/suites/basic/test/counter.js (2)
5-18: LGTM: Utility functionexpectRevert.The utility function for handling promise rejections is correctly implemented.
20-100: LGTM: Tests forCountercontract.The tests for the
Countercontract are correctly implemented and cover various scenarios.tests/integration/ledger/mocks/AccountRetriever.go (1)
1-125: Mock implementation looks good.The mock functions for
AccountRetrieverare correctly implemented using themockerylibrary. This setup will facilitate testing by allowing you to simulate different behaviors of theAccountRetrieverinterface.tests/integration/ledger/evmosd_suite_test.go (1)
1-182: Test suite setup is well-structured.The
LedgerTestSuiteis well-organized, leveragingtestifyandginkgofor testing. The setup functions ensure proper initialization of components, and the use of mocks effectively isolates tests from external dependencies.tests/integration/ledger/ledger_test.go (1)
1-234: Comprehensive tests for Ledger CLI and keyring functionality.The tests effectively cover key functionalities, such as adding keys and signing transactions. The use of
ginkgoandgomegaprovides a clear structure and expressive test definitions. The setup and teardown steps ensure a clean test environment.tests/solidity/test-helper.js (2)
7-11: Logger implementation is straightforward and effective.The logger object provides a simple interface for logging warnings, errors, and informational messages using console methods.
13-16: Panic function is effective for handling critical errors.The
panicfunction logs an error message and exits the process, which is suitable for handling critical errors.example_chain/local_node.sh (1)
202-202: Clarify the purpose of the"$TRACE"variable.The inclusion of
"$TRACE"in theosd startcommand suggests it might control tracing or logging behavior. However, it's unclear how this variable is set or what its expected values are. Please provide more context or documentation regarding its usage.
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (16)
tests/solidity/suites/basic/contracts/Counter.sol (1)
6-6: Consider initializing the counter in the constructor.While the counter is initialized to zero by default, explicitly initializing it in a constructor can improve clarity.
contract Counter { + constructor() { + counter = 0; + } uint256 counter = 0;scripts/run-solidity-tests.sh (1)
25-25: Consider adding error handling for the test command.The
yarn testcommand could fail, and it would be beneficial to handle errors gracefully.yarn test --network evmos "$@" +if [ $? -ne 0 ]; then + echo "Tests failed" + exit 1 +fitests/solidity/suites/basic/contracts/Storage.sol (1)
29-31: Clarify the purpose of theshouldRevertfunction.Consider adding a comment to explain the purpose of this function and why it should always revert.
function shouldRevert() pure public { + // This function is intended to always revert for testing purposes require(false, 'This must REVERT'); }tests/solidity/suites/exception/test/revert.js (1)
5-23: Consider adding assertions for initial state verification.While the test case checks the state after operations, it might be beneficial to assert the initial state of the contract to ensure it starts from a known state. This can help catch unexpected initializations.
beforeEach(async () => { revert = await TestRevert.new() + let no = await revert.query_a() + assert.equal(no, '0', 'Initial state of a should be 0') + no = await revert.query_b() + assert.equal(no, '0', 'Initial state of b should be 0') + no = await revert.query_c() + assert.equal(no, '0', 'Initial state of c should be 0') })tests/solidity/suites/opcode/test/opCodes.js (1)
11-20: Enhance test clarity with comments or descriptive assertions.Consider adding comments or more descriptive assertions to clarify what specific behaviors or outcomes are expected from the
test()andtest_stop()methods.tests/solidity/suites/exception/contracts/TestRevert.sol (1)
18-44: Consider explicit error handling or logging intry_set.The try-catch block in the
try_setfunction could benefit from explicit error handling or logging to improve traceability.tests/solidity/suites/basic/test/events.js (1)
15-31: Enhance test clarity with comments or additional assertions.Consider adding comments or more descriptive assertions to clarify the expected event values in the
storeWithEventfunction.tests/integration/ledger/mocks/tendermint.go (1)
23-27: Clarify the purpose of the mock implementation.Consider adding comments to explain the purpose of the
MockTendermintRPCmethods, especially if they are intended to simulate specific behaviors or conditions.tests/solidity/suites/precompiles/package.json (1)
6-11: Consider Adding Script Descriptions.Adding brief descriptions to each script can improve readability and understanding for future developers.
tests/integration/ledger/mocks/registry.go (3)
13-14: Fix Typographical Error.Correct the spelling of "Methods registry" in the comment.
// Methods registrtry for SECP256k1 +// Methods registry for SECP256k1
41-42: Fix Typographical Error.Correct the spelling of "Methods registry" in the comment.
// Methods registrtry for AccountRetriever +// Methods registry for AccountRetriever
49-49: Fix Typographical Error in Comment.Correct the spelling of "types" in the comment.
// original: EnsureExists(client.Context, ypes.AccAddress) error +// original: EnsureExists(client.Context, types.AccAddress) errortests/solidity/suites/basic/test/counter.js (1)
20-100: LGTM! Consider improving error messages.The test cases for the
Countercontract are comprehensive and well-structured.Consider providing more descriptive error messages in assertions to aid debugging.
tests/solidity/test-helper.js (2)
18-85: Consider improving error messages for user-friendliness.The error messages in the
checkTestEnvfunction could be more descriptive to help users understand and resolve issues more easily.
88-149: Enhance logging for skipped tests.Consider providing more detailed logging information when tests are skipped, such as the reason for skipping.
tests/solidity/init-node.sh (1)
34-69: Add comments to explain the significance of each key.Consider adding comments to explain the purpose and significance of each key being imported for better clarity.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
tests/solidity/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (42)
- CHANGELOG.md (1 hunks)
- example_chain/local_node.sh (1 hunks)
- scripts/run-solidity-tests.sh (1 hunks)
- tests/integration/ledger/evmosd_suite_test.go (1 hunks)
- tests/integration/ledger/ledger_test.go (1 hunks)
- tests/integration/ledger/mocks/AccountRetriever.go (1 hunks)
- tests/integration/ledger/mocks/SECP256K1.go (1 hunks)
- tests/integration/ledger/mocks/registry.go (1 hunks)
- tests/integration/ledger/mocks/tendermint.go (1 hunks)
- tests/solidity/.gitattributes (1 hunks)
- tests/solidity/.gitignore (1 hunks)
- tests/solidity/init-node.sh (1 hunks)
- tests/solidity/package.json (1 hunks)
- tests/solidity/suites/basic/contracts/Counter.sol (1 hunks)
- tests/solidity/suites/basic/contracts/EventTest.sol (1 hunks)
- tests/solidity/suites/basic/contracts/Storage.sol (1 hunks)
- tests/solidity/suites/basic/package.json (1 hunks)
- tests/solidity/suites/basic/test/counter.js (1 hunks)
- tests/solidity/suites/basic/test/events.js (1 hunks)
- tests/solidity/suites/basic/test/revert.js (1 hunks)
- tests/solidity/suites/basic/test/storage.js (1 hunks)
- tests/solidity/suites/basic/truffle-config.js (1 hunks)
- tests/solidity/suites/eip1559/package.json (1 hunks)
- tests/solidity/suites/eip1559/test/eip1559.js (1 hunks)
- tests/solidity/suites/eip1559/truffle-config.js (1 hunks)
- tests/solidity/suites/exception/contracts/TestRevert.sol (1 hunks)
- tests/solidity/suites/exception/contracts/test/Migrations.sol (1 hunks)
- tests/solidity/suites/exception/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/exception/package.json (1 hunks)
- tests/solidity/suites/exception/test/revert.js (1 hunks)
- tests/solidity/suites/exception/truffle-config.js (1 hunks)
- tests/solidity/suites/opcode/contracts/Migrations.sol (1 hunks)
- tests/solidity/suites/opcode/contracts/OpCodes.sol (1 hunks)
- tests/solidity/suites/opcode/migrations/1_initial_migration.js (1 hunks)
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js (1 hunks)
- tests/solidity/suites/opcode/package.json (1 hunks)
- tests/solidity/suites/opcode/test/opCodes.js (1 hunks)
- tests/solidity/suites/opcode/truffle-config.js (1 hunks)
- tests/solidity/suites/precompiles/hardhat.config.js (1 hunks)
- tests/solidity/suites/precompiles/package.json (1 hunks)
- tests/solidity/suites/precompiles/test/staking.js (1 hunks)
- tests/solidity/test-helper.js (1 hunks)
Files skipped from review due to trivial changes (11)
- CHANGELOG.md
- tests/integration/ledger/mocks/AccountRetriever.go
- tests/integration/ledger/mocks/SECP256K1.go
- tests/solidity/.gitattributes
- tests/solidity/.gitignore
- tests/solidity/suites/basic/truffle-config.js
- tests/solidity/suites/eip1559/package.json
- tests/solidity/suites/exception/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/1_initial_migration.js
- tests/solidity/suites/opcode/migrations/2_opCodes_migration.js
- tests/solidity/suites/opcode/package.json
Additional comments not posted (41)
tests/solidity/suites/exception/contracts/test/Migrations.sol (1)
1-19: LGTM!The
Migrationscontract is well-structured and follows Solidity best practices for access control using a modifier. The SPDX license identifier is correctly included.tests/solidity/suites/exception/truffle-config.js (1)
1-17: LGTM!The Truffle configuration for the Evmos network is correctly set up with appropriate network settings and compiler version. The comments provide clarity on the configuration.
tests/solidity/suites/opcode/truffle-config.js (1)
1-17: LGTM!The Truffle configuration for the Evmos network is correctly set up with appropriate network settings and compiler version. The comments provide clarity on the configuration.
tests/solidity/suites/exception/package.json (1)
1-26: Configuration is well-structured and meets standards.The package.json file is well-organized, with appropriate scripts and dependencies for testing.
tests/solidity/suites/eip1559/truffle-config.js (1)
1-19: Truffle configuration looks good.The configuration for the Evmos network and Solidity compiler version is appropriate for development purposes.
tests/solidity/suites/basic/package.json (1)
1-26: Package configuration is well-structured.The scripts and dependencies are appropriate for a Solidity test suite. The inclusion of truffle-assertions is beneficial for contract testing.
tests/solidity/suites/opcode/contracts/Migrations.sol (1)
1-23: Migration contract is standard but consider updating the Solidity version pragma.The contract is well-implemented for managing migrations. However, consider updating the Solidity version pragma to a more recent version for compatibility with newer features and security improvements.
-pragma solidity >=0.4.21 <0.6.0; +pragma solidity >=0.6.0 <0.9.0;tests/solidity/suites/exception/test/revert.js (1)
5-23: LGTM! The test logic is clear and concise.The test case effectively checks the revert behavior of the
TestRevertcontract.tests/solidity/package.json (1)
1-37: LGTM! The package configuration is well-structured.The use of workspaces and nohoist is appropriate, and the scripts are standard for a testing setup.
tests/solidity/suites/basic/contracts/EventTest.sol (1)
1-37: LGTM! The contract implementation is clear and effective.The contract demonstrates the use of events well, and the
indexedkeyword is used correctly.tests/solidity/suites/basic/test/storage.js (3)
8-12: Deployment test is well-implemented.The test correctly verifies the deployment of the
Storagecontract.
14-18: Storage test is well-implemented.The test correctly verifies that a value is stored in the
Storagecontract.
20-23: Retrieval test is well-implemented.The test correctly verifies that a value is retrieved from the
Storagecontract.tests/solidity/suites/precompiles/test/staking.js (1)
5-25: Staking test is well-implemented.The test correctly verifies the staking process and the delegation balance.
tests/solidity/suites/basic/test/revert.js (3)
5-18: Utility functionexpectRevertis well-implemented.The function correctly identifies revert exceptions in promises.
22-26: Deployment test is well-implemented.The test correctly verifies the deployment of the
Storagecontract.
28-30: Revert scenario test is well-implemented.The test correctly verifies the revert scenario using the
expectRevertutility function.tests/solidity/suites/opcode/test/opCodes.js (2)
22-30: LGTM! The test for invalid opcode handling is clear and effective.The test correctly checks that an error is thrown for an invalid opcode.
32-40: LGTM! The test for revert behavior is clear and effective.The test correctly checks that an error is thrown for a revert operation.
tests/solidity/suites/exception/contracts/TestRevert.sol (1)
4-16: LGTM! TheStatecontract is simple and effective.The use of
requirein thesetfunction is appropriate for enforcing constraints.tests/solidity/suites/basic/test/events.js (1)
9-13: LGTM! The test for contract deployment is straightforward and effective.The test correctly verifies that the
EventTestcontract is deployed by checking the contract address.tests/integration/ledger/mocks/tendermint.go (1)
1-2: Ensure License Consistency.Verify that the license information is consistent with the project's overall licensing strategy.
tests/solidity/suites/precompiles/package.json (2)
1-5: Verify License Compatibility.Ensure that the GPL-3.0-or-later license is compatible with all dependencies and the overall project.
12-34: Review Dependency Versions.Regularly review and update dependency versions to ensure compatibility and security.
tests/solidity/suites/basic/test/counter.js (1)
5-18: LGTM!The
expectRevertfunction is correctly implemented to handle revert errors.tests/integration/ledger/evmosd_suite_test.go (7)
59-65: LGTM!The
TestLedgerfunction correctly initializes and runs theLedgerTestSuite.
67-81: LGTM!The
SetupTestfunction is well-structured and correctly initializes the necessary components for testing.
83-113: LGTM!The
SetupEvmosAppfunction is well-organized and correctly sets up the application for testing.
115-144: LGTM!The
NewKeyringAndCtxsfunction is comprehensive and correctly sets up the keyring and context for testing.
146-167: LGTM!The
evmosAddKeyCmdfunction is well-structured and correctly configures the command for adding a key.
169-178: LGTM!The
MockKeyringOptionfunction is correctly implemented and provides the necessary configurations for testing.
180-182: LGTM!The
FormatFlagfunction is simple and correctly formats the flag string for command-line usage.tests/integration/ledger/ledger_test.go (2)
56-99: LGTM!The tests for adding a key from the Ledger using the CLI are comprehensive and well-structured.
101-230: LGTM!The tests for signing transactions with the Ledger are well-structured and cover various scenarios.
tests/solidity/test-helper.js (4)
7-11: Logger implementation is clear and effective.The logger object is well-implemented, providing clear methods for logging different levels of messages.
13-16: Panic function is well-implemented.The
panicfunction effectively logs an error message and exits the process.
152-171: PerformTestSuite function is well-implemented.The function correctly handles the execution of test suites and manages process output.
173-187: PerformTests function is effective and clear.The function correctly iterates over and executes test suites, using async/await effectively.
tests/solidity/init-node.sh (1)
1-26: Initial setup and configuration are correct.The script correctly initializes environment variables and checks for necessary dependencies.
tests/solidity/suites/opcode/contracts/OpCodes.sol (2)
3-8: Test1 contract implementation is correct.The
Test1contract correctly implements a function to compare two addresses.
10-299: Review the necessity and safety of assembly operations.The
OpCodescontract uses low-level assembly instructions. Ensure that each operation is necessary and safe, considering potential risks and optimizations.
| accounts: [ | ||
| '0x88CBEAD91AEE890D27BF06E003ADE3D4E952427E88F88D31D61D3EF5E5D54305', | ||
| '0x3B7955D25189C99A7468192FCBC6429205C158834053EBE3F78F4512AB432DB9' | ||
| ] |
There was a problem hiding this comment.
Security Concern: Handle private keys securely.
The private keys are hardcoded in the configuration file, which is a security risk. Consider using environment variables or a secure vault to manage sensitive information.
| ? accounts[1] | ||
| : '0x0000000000000000000000000000000000000000', |
There was a problem hiding this comment.
Remove unnecessary conditional operator.
The conditional operator is redundant since accounts[1] is always truthy in this context. Simplify the code by directly using accounts[1].
Apply this diff to simplify the code:
from: accounts[0],
- to: accounts[1]
- ? accounts[1]
- : '0x0000000000000000000000000000000000000000',
+ to: accounts[1],Committable suggestion was skipped due to low confidence.
| function add() public { | ||
| counter++; | ||
| emit Added(counter); | ||
| emit Changed(counter); | ||
| } |
There was a problem hiding this comment.
Consider adding access control to the add function.
If the add function should not be publicly accessible, consider adding access control to restrict who can call it.
function add() public {
+ // Add access control logic here if neededCommittable suggestion was skipped due to low confidence.
| function subtract() public { | ||
| require(counter > 0, ERROR_TOO_LOW); | ||
| counter--; | ||
| emit Changed(counter); | ||
| } |
There was a problem hiding this comment.
Consider adding access control to the subtract function.
If the subtract function should not be publicly accessible, consider adding access control to restrict who can call it.
function subtract() public {
+ // Add access control logic here if neededCommittable suggestion was skipped due to low confidence.
| function getCounter() public view returns (uint256) { | ||
| return counter; | ||
| } |
There was a problem hiding this comment.
Consider marking the getCounter function as external.
If the getCounter function is intended for external use only, consider marking it as external to optimize gas usage.
function getCounter() public view returns (uint256) {
+ // Consider changing `public` to `external` if appropriateCommittable suggestion was skipped due to low confidence.
| async function main () { | ||
| const runConfig = checkTestEnv() | ||
| const allTests = loadTests(runConfig) | ||
|
|
||
| console.log(`Running Tests: ${allTests.join()}`) | ||
|
|
||
| const proc = await setupNetwork({ runConfig, timeout: 50000 }) | ||
| await performTests({ allTests, runConfig }) | ||
|
|
||
| if (proc) { | ||
| proc.kill() | ||
| } | ||
| process.exit(0) | ||
| } | ||
|
|
||
| // Add handler to exit the program when UnhandledPromiseRejection | ||
|
|
||
| process.on('unhandledRejection', (e) => { | ||
| console.error(e) | ||
| process.exit(-1) | ||
| }) | ||
|
|
||
| main() |
There was a problem hiding this comment.
Consider adding error handling for test execution in main.
Adding error handling for potential issues during test execution in the main function could improve robustness.
| # Change parameter token denominations to aevmos | ||
| jq '.app_state["staking"]["params"]["bond_denom"]="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state["gov"]["deposit_params"]["min_deposit"][0]["denom"]="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state["gov"]["params"]["min_deposit"][0]["denom"]="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state["evm"]["params"]["evm_denom"]="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state["mint"]["params"]["mint_denom"]="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
|
|
||
| # Enable precompiles in EVM params | ||
| jq '.app_state["evm"]["params"]["active_static_precompiles"]=["0x0000000000000000000000000000000000000100","0x0000000000000000000000000000000000000400","0x0000000000000000000000000000000000000800","0x0000000000000000000000000000000000000801","0x0000000000000000000000000000000000000802","0x0000000000000000000000000000000000000803","0x0000000000000000000000000000000000000804"]' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
|
|
||
| # set gov proposing && voting period | ||
| jq '.app_state.gov.deposit_params.max_deposit_period="10s"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state.gov.voting_params.voting_period="10s"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
|
|
||
| # When upgrade to cosmos-sdk v0.47, use gov.params to edit the deposit params | ||
| # check if the 'params' field exists in the genesis file | ||
| if jq '.app_state.gov.params != null' "$GENESIS" | grep -q "true"; then | ||
| jq '.app_state.gov.params.min_deposit[0].denom="aevmos"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state.gov.params.max_deposit_period="10s"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| jq '.app_state.gov.params.voting_period="10s"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
| fi | ||
|
|
||
| # Set gas limit in genesis | ||
| jq '.consensus_params.block.max_gas="10000000"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
|
|
||
| # Set base fee in genesis | ||
| jq '.app_state["feemarket"]["params"]["base_fee"]="'${BASEFEE}'"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" | ||
|
|
||
| # disable produce empty block | ||
| sed -i.bak 's/create_empty_blocks = true/create_empty_blocks = false/g' "$CONFIG_TOML" | ||
|
|
||
| # Allocate genesis accounts (cosmos formatted addresses) | ||
| osd genesis add-genesis-account "$(osd keys show "$VAL_KEY" -a --keyring-backend "$KEYRING" --home "$CHAINDIR")" 100000000000000000000000000aevmos --keyring-backend "$KEYRING" --home "$CHAINDIR" | ||
| osd genesis add-genesis-account "$(osd keys show "$USER1_KEY" -a --keyring-backend "$KEYRING" --home "$CHAINDIR")" 1000000000000000000000aevmos --keyring-backend "$KEYRING" --home "$CHAINDIR" | ||
| osd genesis add-genesis-account "$(osd keys show "$USER2_KEY" -a --keyring-backend "$KEYRING" --home "$CHAINDIR")" 1000000000000000000000aevmos --keyring-backend "$KEYRING" --home "$CHAINDIR" | ||
| osd genesis add-genesis-account "$(osd keys show "$USER3_KEY" -a --keyring-backend "$KEYRING" --home "$CHAINDIR")" 1000000000000000000000aevmos --keyring-backend "$KEYRING" --home "$CHAINDIR" | ||
| osd genesis add-genesis-account "$(osd keys show "$USER4_KEY" -a --keyring-backend "$KEYRING" --home "$CHAINDIR")" 1000000000000000000000aevmos --keyring-backend "$KEYRING" --home "$CHAINDIR" | ||
|
|
||
| # set custom pruning settings | ||
| if [ "$PRUNING" = "custom" ]; then | ||
| sed -i.bak 's/pruning = "default"/pruning = "custom"/g' "$APP_TOML" | ||
| sed -i.bak 's/pruning-keep-recent = "0"/pruning-keep-recent = "2"/g' "$APP_TOML" | ||
| sed -i.bak 's/pruning-interval = "0"/pruning-interval = "10"/g' "$APP_TOML" | ||
| fi | ||
|
|
||
| # make sure the localhost IP is 0.0.0.0 | ||
| sed -i.bak 's/localhost/0.0.0.0/g' "$CONFIG_TOML" | ||
| sed -i.bak 's/127.0.0.1/0.0.0.0/g' "$APP_TOML" |
There was a problem hiding this comment.
Consider making the chain directory configurable.
Making the chain directory configurable would enhance the flexibility of the script.
This PR adds the Solidity tests as well as some ledger related tests from the Evmos repository.
Added tests can be run with:
make test-solidity make test-unit # for the added Go testsSummary by CodeRabbit
New Features
Bug Fixes
Documentation
.gitattributesfor better syntax highlighting and language recognition for Solidity files.Chores
.gitignorefiles to manage unnecessary files in testing directories.