test: add service for e2e CCTP gateway tests #60
Conversation
WalkthroughAdds smart-contract build tooling and ABI/bindings, a new e2e gateway service with types (signer, permit, generic contract), an example deposit-for-burn flow and configs/docs, updates CI/Makefile to run tests under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Example
participant Config as Config Loader
participant Service as E2E Service
participant RPC as Ethereum RPC
participant USDC as USDC Contract
participant Gateway as OrbiterGateway
User->>Config: Load TOML
Config-->>User: EthereumConfig
User->>Service: NewService(ctx, cfg)
Service->>Service: setSigner / setClient / setContracts
Service-->>User: Service ready
User->>Service: SignerBalance(ctx)
Service->>RPC: BalanceAt(signer)
RPC-->>Service: ETH balance
User->>Service: SignerUSDCBalance(ctx)
Service->>USDC: BalanceOf(signer)
USDC-->>Service: USDC balance
User->>Service: Build Permit & Sign
User->>Service: TxOpts(ctx)
Service->>RPC: PendingNonceAt / SuggestGasPrice
Service-->>User: Tx options
User->>Service: DepositForBurnWithOrbiter(txOpts,...)
Service->>Gateway: depositForBurnWithOrbiter(...)
Gateway-->>Service: tx submitted
User->>Service: WaitForTransaction(txHash)
Service->>RPC: Poll receipt
RPC-->>Service: Receipt
Service->>Service: ParseDepositForBurnEvents(receipt)
Service-->>User: Parsed events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-08-25T10:35:00.822ZApplied to files:
📚 Learning: 2025-08-25T10:35:00.822ZApplied to files:
🧬 Code graph analysis (1)e2e/gateway/types/gateway.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0xstepit
force-pushed
the
stepit/ethereum-gateway
branch
from
17bc30b to
c8956fe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
e2e/gateway/config/config.go (1)
29-29: Consider making ConfigDir configurable.The hardcoded
"./"value limits flexibility for different environments and testing scenarios. While this works for the current use case, consider whether this should be a variable or removed in favor of requiring callers to provide full paths.e2e/gateway/service/deposit.go (1)
41-60: Simplify return signature or add error handling.The function signature returns
errorbut always returnsnilat Line 59. Either:
- Remove the error return if parsing errors should always be ignored (current behavior), or
- Return an error when parsing fails unexpectedly (e.g., accumulate and return if no events matched but logs were present).
The current approach where parse errors at lines 52-54 are silently ignored via
continueis reasonable for filtering non-matching logs, but the unused error return type creates ambiguity.e2e/gateway/examples/deposit_for_burn_orbiter.go (1)
40-40: Document the payload constant.The hardcoded payload hex string lacks explanation. For an example program, adding a comment describing what this payload represents (e.g., test Orbiter payload structure, sample cross-chain message, etc.) would improve clarity for users learning from this code.
e2e/gateway/config/ethereum.go (1)
57-66: Clarify TokenMessenger validation.The
TokenMessengerfield at Line 53 is not validated, whileUSDCandGatewayare required. IfTokenMessengeris optional, consider documenting this with a comment. If it's required, add validation. Additionally, consider validating that the provided strings are valid Ethereum addresses (e.g., usingcommon.IsHexAddress()) rather than just checking for non-empty values.e2e/gateway/service/balance.go (1)
46-48: Align SignerUSDCBalance with SignerBalance pattern.
SignerBalance(Line 38) delegates to theBalancehelper, butSignerUSDCBalancedirectly callss.usdc.Instance().BalanceOf()instead of using theUSDCBalancehelper defined at Line 42. For consistency, consider refactoring:Apply this diff:
func (s *Service) SignerUSDCBalance(ctx context.Context) (*big.Int, error) { - return s.usdc.Instance().BalanceOf(nil, s.signer.Address()) + return s.USDCBalance(ctx, s.signer.Address()) }e2e/gateway/types/signer.go (1)
38-54: Document expected private key format.The
crypto.HexToECDSAfunction at Line 39 expects a hex string without the "0x" prefix. Consider adding a comment or parameter documentation to clarify this requirement for callers, as providing a "0x"-prefixed key will result in a parsing error.Example:
+// NewSigner creates a new Signer from a hex-encoded private key (without "0x" prefix). func NewSigner(privateKeyHex string) (*Signer, error) {e2e/gateway/service/service.go (1)
119-119: Consider making gas limit configurable.The hard-coded
GasLimitof 300,000 may not be suitable for all transaction types. Complex contract interactions withdepositForBurnWithOrbitermight require more gas, while simpler operations could use less.Consider adding a gas limit parameter to the configuration or making it dynamically estimated:
+ gasLimit, err := s.client.EstimateGas(ctx, ethereum.CallMsg{ + From: signer.Address(), + To: /* target address */, + Value: big.NewInt(0), + Data: /* transaction data */, + }) + if err != nil { + // Fallback to default + gasLimit = 300000 + } - txOpts.GasLimit = uint64(300000) + txOpts.GasLimit = gasLimitAlternatively, add a
GasLimitfield toEthereumConfigif estimation is not feasible for all use cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.github/workflows/e2e-tests.yaml(2 hunks)Makefile(1 hunks)contracts/Makefile(1 hunks)contracts/abi/OrbiterGatewayCCTP.abi(1 hunks)contracts/src/OrbiterGatewayCCTP.sol(1 hunks)e2e/gateway/README.md(1 hunks)e2e/gateway/config.example.toml(1 hunks)e2e/gateway/config/config.go(1 hunks)e2e/gateway/config/ethereum.go(1 hunks)e2e/gateway/examples/config_sepolia_local.template.toml(1 hunks)e2e/gateway/examples/config_sepolia_local.toml(1 hunks)e2e/gateway/examples/deposit_for_burn_orbiter.go(1 hunks)e2e/gateway/service/balance.go(1 hunks)e2e/gateway/service/client.go(1 hunks)e2e/gateway/service/deposit.go(1 hunks)e2e/gateway/service/permit.go(1 hunks)e2e/gateway/service/service.go(1 hunks)e2e/gateway/types/contracts.go(1 hunks)e2e/gateway/types/gateway.go(1 hunks)e2e/gateway/types/permit.go(1 hunks)e2e/gateway/types/signer.go(1 hunks)e2e/interchaintests/ibc_to_cctp_test.go(1 hunks)e2e/interchaintests/ibc_to_hyperlane_test.go(1 hunks)e2e/interchaintests/setup.go(1 hunks)e2e/interchaintests/utils.go(1 hunks)e2e/interchaintests/utils_queries.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Go code must be formatted and pass golangci-lint
All Go files must include license headers
Files:
e2e/interchaintests/utils.goe2e/gateway/service/deposit.goe2e/gateway/examples/deposit_for_burn_orbiter.goe2e/gateway/service/permit.goe2e/gateway/types/permit.goe2e/interchaintests/setup.goe2e/gateway/service/client.goe2e/gateway/config/config.goe2e/interchaintests/utils_queries.goe2e/gateway/types/gateway.goe2e/gateway/types/signer.goe2e/gateway/config/ethereum.goe2e/interchaintests/ibc_to_hyperlane_test.goe2e/gateway/types/contracts.goe2e/interchaintests/ibc_to_cctp_test.goe2e/gateway/service/balance.goe2e/gateway/service/service.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Write unit tests using the testCases pattern
Name each test case as "success - " for success cases and "error - " for error cases; omit the prefix when no error is tested
For error-checking cases, include an expError string field in the test case and assert with require.ErrorContains(t, tC.expError, err)
Files:
e2e/interchaintests/ibc_to_hyperlane_test.goe2e/interchaintests/ibc_to_cctp_test.go
🧠 Learnings (2)
📚 Learning: 2025-08-25T10:35:00.822Z
Learnt from: CR
Repo: noble-assets/orbiter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:35:00.822Z
Learning: Applies to **/*_test.go : Write unit tests using the testCases pattern
Applied to files:
e2e/interchaintests/utils.goe2e/interchaintests/setup.goe2e/interchaintests/utils_queries.goe2e/interchaintests/ibc_to_hyperlane_test.goe2e/interchaintests/ibc_to_cctp_test.go
📚 Learning: 2025-08-18T07:26:16.709Z
Learnt from: 0xstepit
Repo: noble-assets/orbiter PR: 15
File: autocli.go:26-35
Timestamp: 2025-08-18T07:26:16.709Z
Learning: In the orbiter codebase, service descriptors follow different naming conventions depending on their package location: API packages (api/*/v1) use Msg_ServiceDesc/Query_ServiceDesc (uppercase 'S'), while types packages (types/component/*) use Msg_serviceDesc/Query_serviceDesc (lowercase 's'). The autocli.go correctly uses both conventions depending on which service descriptors it's referencing.
Applied to files:
e2e/gateway/service/service.go
🔇 Additional comments (6)
e2e/gateway/types/permit.go (1)
85-100: LGTM: Correct EIP-712 implementation.The
Digest()method correctly implements EIP-712 structured data hashing with proper field encoding (32-byte left-padded slots), struct hashing, and final digest computation with the version prefix and domain separator.e2e/gateway/types/contracts.go (1)
33-33: Fix typo in type name.The type alias
InstanceContructorhas a typo - it should beInstanceConstructor. This is a public type that appears to be used as part of the package API, so correcting it now avoids future breaking changes.Apply this diff:
-type InstanceContructor[T any] = func(common.Address, bind.ContractBackend) (*T, error) +type InstanceConstructor[T any] = func(common.Address, bind.ContractBackend) (*T, error)Note: This will require updating all usages of
InstanceContructorthroughout the codebase.Run the following script to find all usages:
e2e/gateway/service/service.go (2)
37-182: Well-structured service implementation.The Service provides a clean abstraction over Ethereum client interactions with:
- Proper resource management via
Close()- Clear separation of concerns between client, signer, and contract instances
- Consistent error handling and context propagation throughout
The initialization sequence in
NewServicefollows a logical order, and the helper methods provide useful utilities for transaction management.
99-122: Verify thread safety for concurrent transaction submission.The
TxOptsmethod retrieves the nonce viaPendingNonceAt, but this pattern can lead to nonce collisions if multiple goroutines callTxOptsconcurrently before transactions are mined. Each call fetches the same pending nonce, resulting in duplicate nonces and transaction replacement or failure.Run the following script to check if the Service is used concurrently in tests or examples:
If concurrent usage is confirmed, consider implementing nonce management within the Signer type or adding a mutex to serialize nonce retrieval and transaction submission.
contracts/abi/OrbiterGatewayCCTP.abi (1)
1-187: ABI file looks correct.This is a properly formatted ABI JSON defining the
OrbiterGatewayCCTPcontract interface. The structure includes the expected constructor, view functions, mutator function, event, and error definitions that align with the contract's purpose for CCTP gateway operations.e2e/gateway/types/gateway.go (1)
21-22: Auto-generated bindings - no manual review required.This file is correctly marked as generated code and should not be manually edited. Any changes to the contract interface should be made in the Solidity source and ABI, then regenerated using the binding generation workflow.
0xstepit
force-pushed
the
stepit/ethereum-gateway
branch
from
4aafa24 to
0010cf0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
e2e/gateway/examples/config_sepolia_local.template.toml (1)
6-9: ** Scrub secrets from template—replace with clear placeholders.**A prior review flagged this critical security risk; it remains unaddressed. This template contains a real mnemonic and private key (flagged by Gitleaks). Developers will copy these credentials directly, leaking a funded account.
Replace with placeholders and add a comment instructing users to provide their own keys locally:
-# Mnemonic: void modify social cup bone scene pave project high race hello gentle -# Address: 0x048bb14d17E997F11e3D156cc0C7C00b35b3bd20 -# The account holds both ETH and USDC. -private_key = "279c39a4a6c2bd28d6e72df5dd94af2db148ca2e0dd413a0ff970695875e8c69" +# Mnemonic: <insert mnemonic here if you want to document it locally> +# Address: <insert signer address> +# Provide your own key locally; do not commit funded credentials. +private_key = "<insert hex-encoded private key>"e2e/gateway/examples/config_sepolia_local.toml (1)
6-9: ** Remove committed private key and mnemonic—use placeholders only.**A prior review flagged this critical security risk; it remains unaddressed. This file commits a real mnemonic, funded account address, and private key (flagged by Gitleaks as Generic API Key).
Either exclude this concrete file from version control (via
.gitignoreand use only the.template.toml), or replace secrets with placeholders:-# Mnemonic: void modify social cup bone scene pave project high race hello gentle -# Address: 0x048bb14d17E997F11e3D156cc0C7C00b35b3bd20 -# The account holds both ETH and USDC. -private_key = "279c39a4a6c2bd28d6e72df5dd94af2db148ca2e0dd413a0ff970695875e8c69" +# Mnemonic: <insert mnemonic here if you want to document it locally> +# Address: <insert signer address> +# Provide your own key locally; do not commit funded credentials. +private_key = "<insert hex-encoded private key>"Recommended: Remove this concrete
.tomlfile from the repository entirely and rely solely on the.template.tomlfor documentation.contracts/Makefile (1)
3-3: Fix missing lint target dependencyThe
alltarget depends onlint, but nolint:rule exists in this Makefile. Runningmake allwill fail with "No rule to make target 'lint'".Apply this diff to remove the non-existent dependency:
-all: deps compile lint +all: deps compileAlternatively, add a
lint:target if linting is required as part of the build process.
🧹 Nitpick comments (2)
e2e/gateway/service/deposit.go (1)
41-60: Consider logging parsing errorsParsing errors are silently ignored (line 52-54), which could mask legitimate issues like ABI mismatches or corrupted log data during testing. Consider at least logging these errors for debugging purposes.
Example enhancement:
event, err := s.gateway.Instance().ParseDepositForBurnWithOrbiter(*log) if err != nil { // Consider logging: fmt.Printf("Warning: failed to parse log: %v\n", err) continue }e2e/gateway/service/service.go (1)
119-119: Consider making gas limit configurableThe gas limit is hardcoded to 300,000. Different operations may require different gas limits. For e2e testing flexibility, consider making this configurable via the
EthereumConfigor as a parameter.Example:
// In config type EthereumConfig struct { // ... existing fields GasLimit uint64 `toml:"gas_limit"` } // In TxOpts gasLimit := s.cfg.GasLimit if gasLimit == 0 { gasLimit = 300000 // default } txOpts.GasLimit = gasLimit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.github/workflows/e2e-tests.yaml(2 hunks)Makefile(1 hunks)contracts/Makefile(1 hunks)contracts/abi/OrbiterGatewayCCTP.abi(1 hunks)contracts/src/OrbiterGatewayCCTP.sol(1 hunks)e2e/gateway/README.md(1 hunks)e2e/gateway/config.example.toml(1 hunks)e2e/gateway/config/config.go(1 hunks)e2e/gateway/config/ethereum.go(1 hunks)e2e/gateway/examples/config_sepolia_local.template.toml(1 hunks)e2e/gateway/examples/config_sepolia_local.toml(1 hunks)e2e/gateway/examples/deposit_for_burn_orbiter.go(1 hunks)e2e/gateway/service/balance.go(1 hunks)e2e/gateway/service/client.go(1 hunks)e2e/gateway/service/deposit.go(1 hunks)e2e/gateway/service/permit.go(1 hunks)e2e/gateway/service/service.go(1 hunks)e2e/gateway/types/contracts.go(1 hunks)e2e/gateway/types/gateway.go(1 hunks)e2e/gateway/types/permit.go(1 hunks)e2e/gateway/types/signer.go(1 hunks)e2e/interchaintests/ibc_to_cctp_test.go(1 hunks)e2e/interchaintests/ibc_to_hyperlane_test.go(1 hunks)e2e/interchaintests/setup.go(1 hunks)e2e/interchaintests/utils.go(1 hunks)e2e/interchaintests/utils_queries.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- e2e/gateway/README.md
🚧 Files skipped from review as they are similar to previous changes (13)
- .github/workflows/e2e-tests.yaml
- e2e/interchaintests/ibc_to_hyperlane_test.go
- e2e/gateway/service/client.go
- e2e/gateway/examples/deposit_for_burn_orbiter.go
- e2e/interchaintests/utils.go
- Makefile
- e2e/gateway/service/balance.go
- e2e/gateway/config/ethereum.go
- e2e/gateway/config/config.go
- e2e/gateway/service/permit.go
- e2e/interchaintests/ibc_to_cctp_test.go
- e2e/gateway/config.example.toml
- e2e/gateway/types/permit.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Go code must be formatted and pass golangci-lint
All Go files must include license headers
Files:
e2e/interchaintests/utils_queries.goe2e/gateway/types/contracts.goe2e/gateway/types/signer.goe2e/gateway/service/service.goe2e/interchaintests/setup.goe2e/gateway/service/deposit.goe2e/gateway/types/gateway.go
🧠 Learnings (3)
📚 Learning: 2025-08-25T10:35:00.822Z
Learnt from: CR
Repo: noble-assets/orbiter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:35:00.822Z
Learning: Applies to **/*_test.go : Write unit tests using the testCases pattern
Applied to files:
e2e/interchaintests/utils_queries.goe2e/interchaintests/setup.go
📚 Learning: 2025-08-18T07:26:16.709Z
Learnt from: 0xstepit
Repo: noble-assets/orbiter PR: 15
File: autocli.go:26-35
Timestamp: 2025-08-18T07:26:16.709Z
Learning: In the orbiter codebase, service descriptors follow different naming conventions depending on their package location: API packages (api/*/v1) use Msg_ServiceDesc/Query_ServiceDesc (uppercase 'S'), while types packages (types/component/*) use Msg_serviceDesc/Query_serviceDesc (lowercase 's'). The autocli.go correctly uses both conventions depending on which service descriptors it's referencing.
Applied to files:
e2e/gateway/service/service.go
📚 Learning: 2025-08-25T10:35:00.822Z
Learnt from: CR
Repo: noble-assets/orbiter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:35:00.822Z
Learning: Applies to **/*.go : Go code must be formatted and pass golangci-lint
Applied to files:
contracts/Makefile
🧬 Code graph analysis (4)
e2e/gateway/types/signer.go (1)
e2e/interchaintests/utils.go (1)
LeftPadBytes(235-246)
e2e/gateway/service/service.go (6)
e2e/gateway/config/ethereum.go (2)
EthereumConfig(28-33)ContractsConfig(51-55)e2e/gateway/service/client.go (1)
EVMClient(32-41)e2e/gateway/types/signer.go (2)
Signer(33-36)NewSigner(38-54)e2e/gateway/types/contracts.go (2)
Contract(28-31)NewContract(35-50)e2e/gateway/types/gateway.go (2)
OrbiterGatewayCCTP(62-66)NewOrbiterGatewayCCTP(121-127)e2e/gateway/types/fiat_token.go (2)
FiatToken(62-66)NewFiatToken(121-127)
e2e/gateway/service/deposit.go (2)
e2e/gateway/service/service.go (1)
Service(38-50)e2e/gateway/types/gateway.go (1)
OrbiterGatewayCCTPDepositForBurnWithOrbiter(509-513)
e2e/gateway/types/gateway.go (1)
e2e/gateway/types/contracts.go (1)
Contract(28-31)
🪛 checkmake (0.2.2)
contracts/Makefile
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 Gitleaks (8.29.0)
e2e/gateway/examples/config_sepolia_local.template.toml
[high] 9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
e2e/gateway/examples/config_sepolia_local.toml
[high] 9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
e2e/interchaintests/utils_queries.go (1)
21-21: LGTM: Package reorganizationThe package rename from
e2etointerchaintestsaligns with the broader PR reorganization.e2e/interchaintests/setup.go (1)
21-21: LGTM: Package reorganizationThe package rename from
e2etointerchaintestsis consistent with the PR's test restructuring.contracts/src/OrbiterGatewayCCTP.sol (1)
147-153: LGTM: Clean accessor functionThe
destinationCaller()view function provides a standard public getter for the immutableDESTINATION_CALLERvalue. The implementation is clean and properly documented.e2e/gateway/service/deposit.go (1)
32-39: LGTM: Straightforward delegationThe
DepositForBurnWithOrbitermethod cleanly delegates to the gateway instance with the provided parameters.e2e/gateway/types/signer.go (1)
38-90: LGTM: Well-structured Ethereum signerThe
Signerimplementation correctly handles:
- Private key loading and validation
- Address derivation from public key
- Signature generation with proper V normalization (27/28 adjustment)
- Transactor creation for transaction signing
- ABI-compatible signature encoding
The error handling is appropriate and the implementation follows Ethereum standards.
e2e/gateway/types/contracts.go (1)
28-58: LGTM: Clean generic contract wrapperThe generic
Contract[T]wrapper provides a clean abstraction for managing contract instances with type safety. The design properly encapsulates the address and instance, with appropriate accessor methods.e2e/gateway/service/service.go (1)
52-138: LGTM: Well-structured service with comprehensive error handlingThe
Serviceimplementation provides a clean abstraction for Ethereum interactions with:
- Proper initialization and cleanup
- Comprehensive error handling with context
- Transaction receipt validation
- Appropriate accessor methods
The design is well-suited for e2e testing scenarios.
contracts/abi/OrbiterGatewayCCTP.abi (1)
1-186: Generated ABI looks correct.This ABI file is generated from the compiled Solidity contract and appears well-formed. The JSON structure is valid, all Solidity types are correct, and the function/event/error definitions align with the contract implementation.
e2e/gateway/types/gateway.go (6)
1-24: License header and generated code warning are correct.The file includes the required BUSL-1.1 license header and clearly marks itself as generated code that should not be manually edited.
26-59: Imports and metadata are standard abigen output.The imports and metadata definitions follow standard go-ethereum binding patterns. All necessary packages are imported, and the ABI is properly embedded in the MetaData structure.
61-163: Binding types and constructors follow standard patterns.The generated binding types and constructor functions follow go-ethereum's standard patterns. All necessary binding variants (caller, transactor, filterer, sessions, raw) are properly defined with appropriate error handling.
165-418: Raw methods and view function bindings are correctly generated.All view functions from the ABI are present with proper type conversions and error handling. Each function includes the standard three variants (Caller, Session, CallerSession) following abigen conventions.
420-439: Mutating function binding is correctly generated.The
DepositForBurnWithOrbitermethod is properly bound with all three variants and correct parameter types matching the ABI definition.
441-592: Event handling code is correctly generated.The event filtering, watching, and parsing methods for
DepositForBurnWithOrbiterfollow standard go-ethereum patterns with proper iterator implementation, subscription management, and error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
contracts/Makefile (1)
1-1: Update.PHONYdeclaration to include all phony targets.The
.PHONYdeclaration is incomplete. It should list all non-file targets defined in the Makefile. Currently,clean,lint, andgenerate-abiare missing.Apply this diff:
-.PHONY: all deps compile +.PHONY: all deps compile clean lint generate-abiThis prevents make from confusing these targets with real files and ensures correct behavior. This is also consistent with the learnings on code quality standards in the repository.
🧹 Nitpick comments (1)
contracts/Makefile (1)
22-23: Verifygenerate-abitarget dependencies.The
generate-abitarget depends on the build artifact./out/OrbiterGatewayCCTP.sol/OrbiterGatewayCCTP.json. Consider adding a dependency on thecompiletarget to ensure the artifact exists before extraction, or document that this target must be run after compilation.If the artifact is not always guaranteed to exist, you could update the target to:
generate-abi: compile @jq '.abi' ./out/OrbiterGatewayCCTP.sol/OrbiterGatewayCCTP.json > ./abi/OrbiterGatewayCCTP.abiThis ensures the contract is compiled before extracting the ABI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)contracts/Makefile(1 hunks)e2e/gateway/examples/config_sepolia_local.template.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- e2e/gateway/examples/config_sepolia_local.template.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T10:35:00.822Z
Learnt from: CR
Repo: noble-assets/orbiter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:35:00.822Z
Learning: Applies to **/*.go : Go code must be formatted and pass golangci-lint
Applied to files:
contracts/Makefile
🪛 checkmake (0.2.2)
contracts/Makefile
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: prepare
🔇 Additional comments (2)
.gitignore (1)
10-10: LGTM!Adding
config_sepolia_local.tomlto the ignore list is appropriate. Local configuration files with sensitive or environment-specific data should not be version-controlled, especially for e2e testing with local Ethereum nodes. The entry follows the existing pattern in the file and aligns well with the template-based configuration approach described in the PR context.contracts/Makefile (1)
3-3:linttarget dependency is now satisfied.The past review flagged that
all: deps compile lintreferenced a non-existentlinttarget. This issue is now resolved—thelinttarget is properly defined at line 16, somake allwill execute successfully.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Makefile(1 hunks)contracts/Makefile(1 hunks)contracts/abi/OrbiterGatewayCCTP.abi(1 hunks)e2e/gateway/README.md(1 hunks)e2e/gateway/service/balance.go(1 hunks)e2e/gateway/types/contracts.go(1 hunks)e2e/gateway/types/gateway.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/gateway/service/balance.go
- Makefile
- e2e/gateway/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Go code must be formatted and pass golangci-lint
All Go files must include license headers
Files:
e2e/gateway/types/contracts.goe2e/gateway/types/gateway.go
🧠 Learnings (1)
📚 Learning: 2025-08-25T10:35:00.822Z
Learnt from: CR
Repo: noble-assets/orbiter PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:35:00.822Z
Learning: Applies to **/*.go : Go code must be formatted and pass golangci-lint
Applied to files:
contracts/Makefile
🧬 Code graph analysis (1)
e2e/gateway/types/gateway.go (1)
e2e/gateway/types/contracts.go (1)
Contract(28-31)
🪛 checkmake (0.2.2)
contracts/Makefile
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 GitHub Actions: Check Licenses
e2e/gateway/types/gateway.go
[error] 1-1: License header check failed: 1 file does not have the correct license header: ./e2e/gateway/types/gateway.go.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
contracts/Makefile (1)
9-23: LGTM! Build targets are well-structured.The Makefile targets follow a clear workflow: install deps with
bun, compile contracts withforge, format withforge fmt, and extract the ABI withjq. Thecleantarget properly removes both forge artifacts and node_modules.e2e/gateway/types/contracts.go (1)
28-58: LGTM! Clean generic contract wrapper.The generic
Contract[T]wrapper provides a type-safe abstraction for Ethereum contract instances. The constructor properly validates the contract by calling the providedInstanceConstructor, and the accessors follow Go conventions.contracts/abi/OrbiterGatewayCCTP.abi (1)
1-196: ABI artifact looks well-formed.This is a generated ABI file produced by the Makefile's
generate-abitarget. The JSON structure is valid and contains the expected contract interface elements (constructor, view functions, mutators, events, and errors).
| @@ -0,0 +1,23 @@ | |||
| .PHONY: all deps compile clean generate-abi | |||
There was a problem hiding this comment.
Add lint to the .PHONY declaration.
The .PHONY declaration on Line 1 is missing the lint target, even though lint is defined at Line 16-17 and used as a dependency in the all target.
Apply this diff:
-.PHONY: all deps compile clean generate-abi
+.PHONY: all deps compile clean generate-abi lint🧰 Tools
🪛 checkmake (0.2.2)
[warning] 1-1: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
In contracts/Makefile around line 1, the .PHONY declaration misses the lint
target (lint is defined at lines 16-17 and used by the all target); update the
.PHONY line to include lint so it reads something like ".PHONY: all deps compile
clean generate-abi lint" (add the lint token separated by a space) to ensure the
lint target is treated as phony.
There was a problem hiding this comment.
@MalteHerrmann Thank you for the confirmation! 👍
MalteHerrmann
left a comment
MalteHerrmann
left a comment
There was a problem hiding this comment.
leaving some comments even tho this might not be finished 🙏 had these open for some time haha
| /** | ||
| * @notice Returns the zero left-padded bytes of the address used for the destination caller. | ||
| * @return bytes32 Bytes associated with the destination caller address on Noble. | ||
| */ | ||
| function destinationCaller() public view returns (bytes32) { | ||
| return DESTINATION_CALLER; | ||
| } |
There was a problem hiding this comment.
since DESTINATION_CALLER is a public variable it can already be retrieved with a getter -- if you want to add the getter here, we can move the var to be private, which would make sense anyways no?
| @@ -0,0 +1,23 @@ | |||
| .PHONY: all deps compile clean generate-abi | |||
| generate-abi: | ||
| @jq '.abi' ./out/OrbiterGatewayCCTP.sol/OrbiterGatewayCCTP.json > ./abi/OrbiterGatewayCCTP.abi |
There was a problem hiding this comment.
nit: generate-abi is misleading here because the compilation already needs to have happened at this point -- maybe rather call it extract-abi?
| func (c *Config) Validate() error { | ||
| if err := c.Ethereum.Validate(); err != nil { | ||
| return fmt.Errorf("ethereum configuration error: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
could add the nil check here too
| func (c *Config) Validate() error { | |
| if err := c.Ethereum.Validate(); err != nil { | |
| return fmt.Errorf("ethereum configuration error: %w", err) | |
| } | |
| return nil | |
| } | |
| func (c *Config) Validate() error { | |
| if c == nil { | |
| return errors.New("nil config") | |
| } | |
| if err := c.Ethereum.Validate(); err != nil { | |
| return fmt.Errorf("ethereum configuration error: %w", err) | |
| } | |
| return nil | |
| } |
| func (c *EthereumConfig) Validate() error { | ||
| if c.RPCEndpoint == "" { | ||
| return fmt.Errorf("ethereum.rpc_endpoint is required") | ||
| } | ||
| if c.PrivateKey == "" { | ||
| return fmt.Errorf("ethereum.private_key is required") | ||
| } | ||
|
|
||
| if err := c.Contracts.Validate(); err != nil { | ||
| return fmt.Errorf("ethereum.contracts configuration error: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
the ethereum config is not stored as a pointer so why specify the Validate method on the pointer receiver?
Using the value receiver we don't need the nil checks
| func (c *EthereumConfig) Validate() error { | |
| if c.RPCEndpoint == "" { | |
| return fmt.Errorf("ethereum.rpc_endpoint is required") | |
| } | |
| if c.PrivateKey == "" { | |
| return fmt.Errorf("ethereum.private_key is required") | |
| } | |
| if err := c.Contracts.Validate(); err != nil { | |
| return fmt.Errorf("ethereum.contracts configuration error: %w", err) | |
| } | |
| return nil | |
| } | |
| func (c EthereumConfig) Validate() error { | |
| if c.RPCEndpoint == "" { | |
| return fmt.Errorf("ethereum.rpc_endpoint is required") | |
| } | |
| if c.PrivateKey == "" { | |
| return fmt.Errorf("ethereum.private_key is required") | |
| } | |
| if err := c.Contracts.Validate(); err != nil { | |
| return fmt.Errorf("ethereum.contracts configuration error: %w", err) | |
| } | |
| return nil | |
| } |
| Gateway string `toml:"gateway"` | ||
| } | ||
|
|
||
| func (c *ContractsConfig) Validate() error { |
There was a problem hiding this comment.
ditto
| func (c *ContractsConfig) Validate() error { | |
| func (c ContractsConfig) Validate() error { |
| if c.PrivateKey == "" { | ||
| return fmt.Errorf("ethereum.private_key is required") | ||
| } |
There was a problem hiding this comment.
this could even go further and check if it can be decoded from hex
| if c.PrivateKey == "" { | |
| return fmt.Errorf("ethereum.private_key is required") | |
| } | |
| if _, err := hex.DecodeFromString(c.PrivateKey); c.PrivateKey == "" || err != nil { | |
| return errors.New("invalid ethereum.private_key") | |
| } |
| if c.USDC == "" { | ||
| return fmt.Errorf("contracts.usdc is required") | ||
| } | ||
| if c.Gateway == "" { | ||
| return fmt.Errorf("contracts.gateway is required") | ||
| } |
There was a problem hiding this comment.
token messenger is not validated here; also we could check for valid hex addresses
| if c.USDC == "" { | |
| return fmt.Errorf("contracts.usdc is required") | |
| } | |
| if c.Gateway == "" { | |
| return fmt.Errorf("contracts.gateway is required") | |
| } | |
| if c.USDC == "" || !ethcommon.IsHexAddress(c.USDC) { | |
| return errors.New("invalid contracts.usdc") | |
| } | |
| if c.TokenMessenger == "" || !ethcommon.IsHexAddress(c.TokenMessenger) { | |
| return errors.New("invalid contracts.usdc") | |
| } | |
| if c.Gateway == "" || !ethcommon.IsHexAddress(c.Gateway) { | |
| return errors.New("invalid contracts.gateway") | |
| } |
|
|
||
| const ( | ||
| permitDuration = 60 * 60 * 24 | ||
| payload = "1b8e010064ca0e8337064bf0cca84a2a084f4c2f787a482707a2373d5ce4a45f40d23c818c9a071eb209427ec0ba1eb86f6861e878cb1024c4d14c1709c5327c1726427c6c37438b0b50fa93a9b172426c290415880d86e0b86008e241343a99a0c99a26c189491a4c4ab89448e1a93487c069282bd36a4a06afb26c466a7abc7d0f48a0ee401ca4f1f1781249e16a624a7a3292989cc2a29d647c2aa51028cfa6a1929cdc697fc5d01f4c43ba537849a793923a295cd1043a3d351e575945e5e99454012550928074c8feb091e359430b63c713862025991a334623e9b05c63f7aac05f1fdc99a27f942eb7d8ad0e7e59723c1bf9c2f8ecef379df71cbb0abf3b47ececf7f49f79fc6dba28fb3f5ab77461b523e1fbf040ffe9850be76e0c791b879d7f3ea00c06fa9f13cf64ecfb4d6948d5ec87ade6918be3d74053409ecba90658f2bb5e5dfb35d138213e3ed30a9848424fb7a371427c3c" |
There was a problem hiding this comment.
comment would be nice how this payload was generated or what it represents
This PR adds a service to manually test the CCTP Gateway contracts deployed on a running ethereum node.
Closes ENG-671
Summary by CodeRabbit
New Features
Documentation
Chores