fix: wallet tx nonce & contract deployment#836
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors nonce retrieval in Rust by requiring an explicit wallet address stored on EnclaveContract for write-capable instances. Adjusts provider filler ordering, updates factory to set wallet_address for write/read paths, and switches write calls to the new nonce helper. Updates ImageID PROGRAM_ID constants, deployment metadata JSON, and adds deployment logging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Factory as EnclaveContractFactory
participant Contract as EnclaveContract<ReadWrite>
participant Provider
Client->>Factory: create_write(signer)
Factory->>Factory: extract signer wallet address
Factory-->>Client: EnclaveContract{ wallet_address = Some(addr) }
rect rgba(230,245,255,0.5)
note right of Client: Write operation (e.g., publish_input)
Client->>Contract: write_op(params)
alt wallet_address is Some(addr)
Contract->>Provider: get_transaction_count(addr)
Provider-->>Contract: next_nonce
Contract->>Provider: send tx with nonce
Provider-->>Contract: tx hash/receipt
Contract-->>Client: result
else wallet_address is None
Contract-->>Client: error: missing wallet address
end
end
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/CRISP/contracts/ImageID.sol (1)
22-22: Address formatting inconsistency between auto-generated files.The
PROGRAM_IDconstant uses single-line formatting here, whilecrates/support/contracts/ImageID.sol(lines 22-25) uses multi-line formatting for the same constant. Since both files are marked as automatically generated, this inconsistency suggests different code generation templates or tooling.Consider standardizing the formatting across all auto-generated
ImageID.solfiles for consistency and maintainability.crates/evm-helpers/src/contracts.rs (1)
350-353: Consistent and correct nonce management pattern.All write methods correctly:
- Hold
NONCE_LOCKfor the entire transaction lifecycle (preventing concurrent nonce conflicts)- Validate
wallet_addressis present with appropriate error handling- Retrieve the next nonce explicitly for the wallet address
Optional: Consider a more descriptive error message:
-.ok_or_else(|| eyre::eyre!("No wallet address configured"))?; +.ok_or_else(|| eyre::eyre!("No wallet address configured. Write operations require a wallet address. Use EnclaveContractFactory::create_write() to create a write-capable contract instance."))?;Also applies to: 377-380, 391-394, 405-408, 424-427, 445-448
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
crates/evm-helpers/src/contracts.rs(11 hunks)crates/support/contracts/ImageID.sol(1 hunks)examples/CRISP/contracts/ImageID.sol(1 hunks)packages/enclave-contracts/deployed_contracts.json(1 hunks)packages/enclave-contracts/scripts/deployEnclave.ts(5 hunks)packages/enclave-contracts/scripts/deployMocks.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/enclave-contracts/scripts/deployMocks.ts (3)
packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts (1)
deployAndSaveMockComputeProvider(15-48)packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts (1)
deployAndSaveMockDecryptionVerifier(15-49)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts (1)
deployAndSaveMockInputValidator(15-45)
crates/evm-helpers/src/contracts.rs (1)
crates/evm/src/helpers.rs (3)
provider(71-73)new(63-69)new(86-88)
⏰ 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). (7)
- GitHub Check: build_sdk
- GitHub Check: Build & Push Image
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: rust_unit
- GitHub Check: test_net
- GitHub Check: test_contracts
🔇 Additional comments (10)
crates/support/contracts/ImageID.sol (1)
22-25: Confirm program ID update and deployment
- No references to the old
PROGRAM_IDwere found in.sol,.rs,.json, or.tsfiles. Ensure this new ID matches your zkVM build, update any deployed contracts, and validate integration tests.packages/enclave-contracts/scripts/deployMocks.ts (1)
26-41: LGTM! Deployment logging improves observability.The added console.log statements provide clear progress indicators for each deployment step, making it easier to track deployment progress and debug issues. The log messages are well-placed immediately before their corresponding deployment calls.
packages/enclave-contracts/scripts/deployEnclave.ts (1)
35-88: LGTM! Consistent deployment progress logging.The added console.log statements provide clear visibility into the deployment process, including Enclave, registry components, and mocks. This aligns with the logging pattern in deployMocks.ts and will aid in troubleshooting deployment issues.
packages/enclave-contracts/deployed_contracts.json (1)
1-49: Verify deployment metadata accuracy
Cross-references look correct (Enclave → CiphernodeRegistry → NaiveRegistryFilter; MockE3Program → MockInputValidator), but please confirm on-chain code is present. Locally run (noskip_cloning) with your Etherscan API key:export ETHERSCAN_API_KEY=your_key_here ENCLAVE=$(jq -r '.sepolia.Enclave.address' packages/enclave-contracts/deployed_contracts.json) REGISTRY=$(jq -r '.sepolia.CiphernodeRegistry.address' packages/enclave-contracts/deployed_contracts.json) FILTER=$(jq -r '.sepolia.NaiveRegistryFilter.address' packages/enclave-contracts/deployed_contracts.json) for addr in "$ENCLAVE" "$REGISTRY" "$FILTER"; do echo "$addr:" $(curl -s "https://api-sepolia.etherscan.io/api?module=proxy&action=eth_getCode&address=$addr&tag=latest&apikey=$ETHERSCAN_API_KEY" | jq -r '.result' | head -c 20) doneEnsure each address returns non-empty bytecode.
crates/evm-helpers/src/contracts.rs (6)
28-38: LGTM! Explicit address parameter improves safety.The new
get_next_noncefunction correctly retrieves the pending nonce for a specific address. Using.pending()ensures that pending transactions are accounted for when determining the next nonce.
177-177: LGTM! Appropriate use ofOption<Address>.Using
Option<Address>correctly distinguishes between read-only contracts (None) and write-capable contracts (Some) that need wallet address for transaction signing and nonce management.
255-255: LGTM! Wallet address properly captured and stored.The wallet address is correctly extracted from the signer and stored in the contract instance for subsequent nonce management operations.
Also applies to: 265-265
282-282: LGTM! Correct initialization for read-only contracts.Setting
wallet_addresstoNoneis appropriate for read-only contract instances that don't perform write operations.
339-373: LGTM! Write methods correctly implement the new nonce management pattern.All six write methods (
request_e3,activate,enable_e3_program,publish_input,publish_ciphertext_output,publish_plaintext_output) follow the correct pattern:
- Acquire the
NONCE_LOCKto serialize nonce operations- Validate
wallet_addressavailability- Retrieve the next nonce explicitly
- Build and send the transaction with the explicit nonce
- Wait for and return the receipt
The implementation is consistent, thread-safe, and maintains proper error handling throughout.
Also applies to: 375-387, 389-401, 403-415, 417-436, 438-457
226-236: NonceFiller is still required for implicit nonce filling
Severalsend().await?calls (e.g. lines 370, 384, 398, 409 incrates/evm-helpers/src/contracts.rs) lack an explicit.nonce()and therefore rely on theNonceFiller. Removing it would break those flows—either retainNonceFilleror add.nonce(...)to every transaction builder beforesend().Likely an incorrect or invalid review comment.
hmzakhalid
changed the title
2 participants
Summary by CodeRabbit