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

feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes #1841

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

Unique-Divine
Copy link
Member

@Unique-Divine Unique-Divine commented Apr 15, 2024

Purpose / Abstract

feat(eth): Collections encoders for bytes, Ethereum addresses, and Ethereum hashes

Summary by CodeRabbit

  • New Features

    • Introduced functionality for encoding and decoding EIP-712 objects in Go.
    • Added support for converting Cosmos transactions into EIP712-compatible TypedData requests.
    • Implemented Ethereum address and hash validation functions.
    • Validated and parsed Ethereum-compatible chain identifiers.
    • Registered interfaces for tendermint concrete client-related implementations.
  • Documentation

    • Updated CONTRIBUTING guidelines and issue templates for better community engagement.
    • Added GitHub Actions workflows for build, lint, security checks, and dependency reviews.
  • Bug Fixes

    • Addressed encoding issues with Ethereum transactions.
  • Chores

    • Improved Docker and GitHub repository configurations to streamline development and deployment processes.
  • Tests

    • Expanded test coverage for cryptographic functions, Ethereum transaction encoding, and chain ID validations.

@Unique-Divine Unique-Divine requested a review from a team as a code owner April 15, 2024 12:11
Copy link
Contributor

coderabbitai bot commented Apr 15, 2024

Walkthrough

This 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

File Path Change Summary
eth/ethereum/eip712/... Adds functionality for EIP-712 encoding, decoding, and message handling for Ethereum transactions.
eth/types/assert.go Introduces functions for Ethereum address and hash validation and management.
eth/types/chain_id.go Adds Ethereum chain ID validation and parsing capabilities.
eth/types/hdpath.go Introduces features for managing HD paths in Ethereum, including defining constants and path creation.
eth/ethereum/eip712/types.go Implements EIP-712 type creation and management for Ethereum transactions.
eth/ethereum/eip712/preprocess.go Introduces a function to reformat Ledger-signed Cosmos transactions for Ethereum transaction compatibility.
eth/types/codec.go, eth/types/state_encoder_test.go Adds functionality for codec registration and testing for Ethereum storage abstractions.

Poem

🐇✨
Oh, in the realm of Ethereum's might,
Where EIP-712 dances with delight.
Addresses checked, paths well-trod,
Each byte encoded, each test a nod.
Let the blockchain sing, our quest in sight,
As we journey through the code's starry night. 🌟
🐇💻


Recent Review Details

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 410bc14 and 045e547.
Files selected for processing (21)
  • LICENSE.md (2 hunks)
  • eth/crypto/codec/amino.go (1 hunks)
  • eth/crypto/codec/codec.go (1 hunks)
  • eth/crypto/ethsecp256k1/ethsecp256k1.go (1 hunks)
  • eth/crypto/hd/algorithm.go (1 hunks)
  • eth/crypto/keyring/options.go (1 hunks)
  • eth/encoding/codec/codec.go (1 hunks)
  • eth/encoding/config.go (1 hunks)
  • eth/ethereum/eip712/domain.go (1 hunks)
  • eth/ethereum/eip712/eip712.go (1 hunks)
  • eth/ethereum/eip712/eip712_legacy.go (1 hunks)
  • eth/ethereum/eip712/encoding.go (1 hunks)
  • eth/ethereum/eip712/encoding_legacy.go (1 hunks)
  • eth/ethereum/eip712/message.go (1 hunks)
  • eth/ethereum/eip712/preprocess.go (1 hunks)
  • eth/ethereum/eip712/types.go (1 hunks)
  • eth/types/assert.go (1 hunks)
  • eth/types/chain_id.go (1 hunks)
  • eth/types/codec.go (1 hunks)
  • eth/types/hdpath.go (1 hunks)
  • eth/types/state_encoder_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (20)
  • eth/crypto/codec/amino.go
  • eth/crypto/codec/codec.go
  • eth/crypto/ethsecp256k1/ethsecp256k1.go
  • eth/crypto/hd/algorithm.go
  • eth/crypto/keyring/options.go
  • eth/encoding/codec/codec.go
  • eth/encoding/config.go
  • eth/ethereum/eip712/domain.go
  • eth/ethereum/eip712/eip712.go
  • eth/ethereum/eip712/eip712_legacy.go
  • eth/ethereum/eip712/encoding.go
  • eth/ethereum/eip712/encoding_legacy.go
  • eth/ethereum/eip712/message.go
  • eth/ethereum/eip712/preprocess.go
  • eth/ethereum/eip712/types.go
  • eth/types/assert.go
  • eth/types/chain_id.go
  • eth/types/codec.go
  • eth/types/hdpath.go
  • eth/types/state_encoder_test.go
Additional comments not posted (1)
LICENSE.md (1)

189-189: Update the copyright year and company name to reflect current details.


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.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @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.

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: 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
Copy link
Contributor

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.

Suggested change
- [#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

Comment on lines 14 to 17
# Implementation

Do you have ideas regarding the implementation of this feature?
Are you willing to implement this feature?
Copy link
Contributor

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.

Suggested change
# 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.
Copy link
Contributor

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.

Suggested change
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).

Comment on lines 15 to 17
#### Expected behaviour


Copy link
Contributor

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.

Suggested change
#### Expected behaviour
#### Expected behaviour

Comment on lines 18 to 20
#### Actual behaviour


Copy link
Contributor

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.

Suggested change
#### Actual behaviour
#### Actual behaviour

Comment on lines 24 to 30
#### Backtrace

````
[backtrace]
````

When submitting logs: please submit them as text and not screenshots.
Copy link
Contributor

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

Comment on lines 17 to 26
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"
Copy link
Contributor

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.

Suggested change
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"

Comment on lines 28 to 34
## 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.
Copy link
Contributor

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.

Suggested change
## 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.

Comment on lines 36 to 40
## 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.
Copy link
Contributor

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.

Suggested change
## 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.

Comment on lines 20 to 37
// 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())
}
Copy link
Contributor

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

@Unique-Divine Unique-Divine changed the base branch from main to ud/evm April 15, 2024 13:18
@matthiasmatt matthiasmatt merged commit 182ea3a into ud/evm Apr 16, 2024
14 checks passed
@matthiasmatt matthiasmatt deleted the ud/evm-2 branch April 16, 2024 07:20
Unique-Divine added a commit that referenced this pull request Apr 16, 2024
* 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)
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