-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes #1841
Conversation
WalkthroughThis update introduces significant improvements to Ethereum transaction handling within the Nibiru EVM features. It includes enhancements like EIP-712 encoding for transactions, Ethereum address and hash validation functions, Ethereum chain ID parsing, and HD path management. These changes aim to streamline Ethereum-related operations and ensure compatibility with Ethereum standards. Changes
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (21)
Files skipped from review as they are similar to previous changes (20)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
|
||
- [#1837](https://github.com/NibiruChain/nibiru/pull/1837) - feat(eth): protos, eth types, and evm module types | ||
- [#1838](https://github.com/NibiruChain/nibiru/pull/1838) - feat(eth): Go-ethereum, crypto, encoding, and unit tests for evm/types | ||
- [#1841](https://github.com/NibiruChain/nibiru/pull/1841) - feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! The entry effectively documents the addition of collections encoders for bytes, Ethereum addresses, and Ethereum hashes. However, please remove the trailing space at the end of the line for cleanliness.
- feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes
+ feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes
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.
- [#1841](https://github.com/NibiruChain/nibiru/pull/1841) - feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes | |
- [#1841](https://github.com/NibiruChain/nibiru/pull/1841) - feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes |
# Implementation | ||
|
||
Do you have ideas regarding the implementation of this feature? | ||
Are you willing to implement this feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Implementation" section effectively encourages contributors to think about how they might implement the feature and their commitment. Please ensure the file ends with a newline to meet common POSIX standards.
+ \n
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.
# Implementation | |
Do you have ideas regarding the implementation of this feature? | |
Are you willing to implement this feature? | |
# Implementation | |
Do you have ideas regarding the implementation of this feature? | |
Are you willing to implement this feature? | |
assignees: '' | ||
--- | ||
|
||
This should only be used in very rare cases e.g. if you are not 100% sure if something is a bug or asking a question that leads to improving the documentation. For general questions please use [discord](https://discord.gg/nthXNEv) or the Ethereum stack exchange at https://ethereum.stackexchange.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider linking the bare URLs to descriptive text for better readability and to improve the aesthetic of the document. For example:
- For general questions please use [discord](https://discord.gg/nthXNEv) or the Ethereum stack exchange at https://ethereum.stackexchange.com.
+ For general questions, please use [Discord](https://discord.gg/nthXNEv) or [Ethereum Stack Exchange](https://ethereum.stackexchange.com).
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.
This should only be used in very rare cases e.g. if you are not 100% sure if something is a bug or asking a question that leads to improving the documentation. For general questions please use [discord](https://discord.gg/nthXNEv) or the Ethereum stack exchange at https://ethereum.stackexchange.com. | |
This should only be used in very rare cases e.g. if you are not 100% sure if something is a bug or asking a question that leads to improving the documentation. For general questions, please use [Discord](https://discord.gg/nthXNEv) or [Ethereum Stack Exchange](https://ethereum.stackexchange.com). |
geth/.github/ISSUE_TEMPLATE/bug.md
Outdated
#### Expected behaviour | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reduce the consecutive blank lines to maintain a clean and concise template.
-
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.
#### Expected behaviour | |
#### Expected behaviour |
geth/.github/ISSUE_TEMPLATE/bug.md
Outdated
#### Actual behaviour | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduce the consecutive blank lines here as well to keep the template tidy.
-
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.
#### Actual behaviour | |
#### Actual behaviour |
geth/.github/ISSUE_TEMPLATE/bug.md
Outdated
#### Backtrace | ||
|
||
```` | ||
[backtrace] | ||
```` | ||
|
||
When submitting logs: please submit them as text and not screenshots. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify a language for the fenced code block to enable syntax highlighting, which improves readability. Also, ensure the file ends with a newline.
26~ ````
+ ```text
28~ ````
+ ```
+ \n
geth/.github/CONTRIBUTING.md
Outdated
Please make sure your contributions adhere to our coding guidelines: | ||
|
||
* Code must adhere to the official Go | ||
[formatting](https://golang.org/doc/effective_go.html#formatting) guidelines | ||
(i.e. uses [gofmt](https://golang.org/cmd/gofmt/)). | ||
* Code must be documented adhering to the official Go | ||
[commentary](https://golang.org/doc/effective_go.html#commentary) guidelines. | ||
* Pull requests need to be based on and opened against the `master` branch. | ||
* Commit messages should be prefixed with the package(s) they modify. | ||
* E.g. "eth, rpc: make trace configs optional" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust the indentation of the unordered list to align with common markdown practices, and remove trailing spaces to clean up the document.
- 20~ [formatting](https://golang.org/doc/effective_go.html#formatting) guidelines
+ 20~ [formatting](https://golang.org/doc/effective_go.html#formatting) guidelines
- 23~ [commentary](https://golang.org/doc/effective_go.html#commentary) guidelines.
+ 23~ [commentary](https://golang.org/doc/effective_go.html#commentary) guidelines.
- 26~ * E.g. "eth, rpc: make trace configs optional"
+ 26~ * E.g., "eth, rpc: make trace configs optional"
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.
Please make sure your contributions adhere to our coding guidelines: | |
* Code must adhere to the official Go | |
[formatting](https://golang.org/doc/effective_go.html#formatting) guidelines | |
(i.e. uses [gofmt](https://golang.org/cmd/gofmt/)). | |
* Code must be documented adhering to the official Go | |
[commentary](https://golang.org/doc/effective_go.html#commentary) guidelines. | |
* Pull requests need to be based on and opened against the `master` branch. | |
* Commit messages should be prefixed with the package(s) they modify. | |
* E.g. "eth, rpc: make trace configs optional" | |
Please make sure your contributions adhere to our coding guidelines: | |
* Code must adhere to the official Go | |
[formatting](https://golang.org/doc/effective_go.html#formatting) guidelines | |
* Code must be documented adhering to the official Go | |
[commentary](https://golang.org/doc/effective_go.html#commentary) guidelines | |
* Pull requests need to be based on and opened against the `master` branch. | |
* Commit messages should be prefixed with the package(s) they modify. | |
* E.g., "eth, rpc: make trace configs optional" |
geth/.github/CONTRIBUTING.md
Outdated
## Can I have feature X | ||
|
||
Before you submit a feature request, please check and make sure that it isn't | ||
possible through some other means. The JavaScript-enabled console is a powerful | ||
feature in the right hands. Please check our | ||
[Geth documentation page](https://geth.ethereum.org/docs/) for more info | ||
and help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure consistent indentation for the list and remove trailing spaces to enhance the readability and professionalism of the document.
- 31~ possible through some other means. The JavaScript-enabled console is a powerful
+ 31~ possible through some other means. The JavaScript-enabled console is a powerful
- 34~ and help.
+ 34~ and help.
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.
## Can I have feature X | |
Before you submit a feature request, please check and make sure that it isn't | |
possible through some other means. The JavaScript-enabled console is a powerful | |
feature in the right hands. Please check our | |
[Geth documentation page](https://geth.ethereum.org/docs/) for more info | |
and help. | |
## Can I have feature X | |
Before you submit a feature request, please check and make sure that it isn't | |
possible through some other means. The JavaScript-enabled console is a powerful | |
feature in the right hands. Please check our | |
[Geth documentation page](https://geth.ethereum.org/docs/) for more info | |
and help. |
geth/.github/CONTRIBUTING.md
Outdated
## Configuration, dependencies, and tests | ||
|
||
Please see the [Developers' Guide](https://geth.ethereum.org/docs/developers/devguide) | ||
for more details on configuring your environment, managing project dependencies | ||
and testing procedures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust the indentation and remove trailing spaces to maintain a clean and professional appearance in the document.
- 39~ for more details on configuring your environment, managing project dependencies
+ 39~ for more details on configuring your environment, managing project dependencies
- 40~ and testing procedures.
+ 40~ and testing procedures.
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.
## Configuration, dependencies, and tests | |
Please see the [Developers' Guide](https://geth.ethereum.org/docs/developers/devguide) | |
for more details on configuring your environment, managing project dependencies | |
and testing procedures. | |
## Configuration, dependencies, and tests | |
Please see the [Developers' Guide](https://geth.ethereum.org/docs/developers/devguide) | |
for more details on configuring your environment, managing project dependencies | |
and testing procedures. |
eth/types/state_encoder_test.go
Outdated
// encoderDeps: Initializes a database and KV store useful for testing | ||
// abstractions over merklized storage like the `collections.Map` and | ||
// `collections.Item`. | ||
func encoderDeps() (sdkstoretypes.StoreKey, sdk.Context, sdkcodec.BinaryCodec) { | ||
sk := sdk.NewKVStoreKey("mock") | ||
dbm := cmtdb.NewMemDB() | ||
ms := sdkstore.NewCommitMultiStore(dbm) | ||
ms.MountStoreWithDB(sk, sdkstoretypes.StoreTypeIAVL, dbm) | ||
if err := ms.LoadLatestVersion(); err != nil { | ||
panic(err) | ||
} | ||
|
||
return sk, | ||
sdk.Context{}. | ||
WithMultiStore(ms). | ||
WithGasMeter(sdk.NewGasMeter(1_000_000_000)), | ||
sdkcodec.NewProtoCodec(sdkcodectypes.NewInterfaceRegistry()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The encoderDeps
function appears to be unused. If it is indeed not used, consider removing it to clean up the codebase. Additionally, organize the imports to follow Go conventions, which can be automatically done using goimports
.
#!/bin/bash
# Verify the usage of `encoderDeps` in the test suite
rg --type go 'encoderDeps' --files-with-matches
# Suggest running goimports
goimports -w eth/types/state_encoder_test.go
* feat(eth): protos, eth types, and evm module types * chore: changelog * feat(eth): unit tests for types, crypto, encoding (#1838) * feat(eth): unit tests for types, crypto, encoding ## Impl `go-ethereum` Adds `go-ethereum` fork with an `Interpreter` interface for running smart contracts in integration tests. ```go type Interpreter interface { // Run loops and evaluates the contract's code with the given input data and returns // the return byte-slice and an error if one occurred. Run(contract *Contract, input []byte, static bool) ([]byte, error) } ``` An `Interpreter` is used to run Ethereum based contracts and will utilize the passed environment to query external sources for state information. The Interpreter will run the byte code VM based on the passed configuration. Changes from go-ethereum v1.11: * Set `callcode` to use `readOnly` mode for precompiled calls. * Remove `IsStateful` function from the `PrecompiledContract` interface, as this remains unused. * Support stateful precompiled contracts. * Add `Address` function to `PrecompiledContract` interface. * Implement custom active precompiles for the EVM. * Define `Interpreter` interface for the EVM. * Move the `JumpTable` defaults to a separate function. * Refactor `Stack` implementation * chore: linter * docs(sample.go): PrivKeyEth * refactor: fix copyright lines and LICENSE entity * feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes (#1841)
Purpose / Abstract
feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores
Tests