Conversation
…etwork When agent restarts with docker-compose, a new network is created. Existing containers (created before restart) may not be connected to this new network. This fix checks and reconnects containers to the correct network when they are reused.
Shorten data URI prefix from 'data:application/json;base64,' (30 bytes) to 'data:;base64,' (13 bytes), saving 17 bytes per on-chain transaction. This optimization reduces L1 calldata costs on Arbitrum L2. The short format is valid per RFC 2397 - MIME type defaults to text/plain.
- Update CommitmentUtils test: remove redundancy, add verifierFee - Update contracts-mock test: add coordinator and verifierFee fields - Update PayloadUtils test: expect short data URI format (data:;base64,) - Update storage test: expect short data URI format
- Update contract ABIs to remove redundancy field from Commitment - Add verifierFee field to Commitment type - Update Coordinator and Router wrappers for new contract interface - Sync typechain generated types with updated ABIs - Update agent-core to handle new Commitment structure - Remove redundancy from ComputeSubscription type - Update tests for new contract structure All 497 tests passing.
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates across the SDK, primarily focusing on contract ABI changes, gas optimization, and Docker container management. The 'redundancy' field has been removed from various contract events and internal data structures (e.g., RequestStartedEvent, Commitment, ComputeSubscription), replaced by a direct check for commitment existence using requestCommitments. A new verifierFee field has been added to relevant contract events and data structures. For gas optimization, a CoordinatorContract wrapper is introduced to support auto-generated access lists for reportComputeResult transactions, and data URI formatting for payloads has been shortened (data:;base64,) to reduce on-chain bytes. Docker container management is enhanced with a new ensureContainerNetwork method to ensure containers are connected to the correct network, improving robustness. Additionally, package versions across the monorepo have been bumped to 0.2.1-alpha.1, and logging for request and container IDs now displays full IDs instead of truncated ones. Review comments highlighted the broad catch block in ContainerManager.ts for potential refinement and emphasized the critical nature of the ABI changes in parseCommitment for data integrity and contract interaction.
| } catch (err: any) { | ||
| // Log but don't fail - container might still work on default network | ||
| console.warn(` ⚠️ Failed to ensure network connection for ${containerName}: ${err.message}`); |
There was a problem hiding this comment.
The catch (err: any) block is quite broad. While the comment indicates a conscious decision not to fail, consider if there are specific Docker-related error types that could be caught to allow for more granular logging or handling of transient vs. permanent issues. This would improve the robustness of the network connection logic.
| containerId: commitment.containerId, | ||
| client: commitment.client, | ||
| wallet: commitment.wallet, | ||
| feeToken: commitment.feeToken, | ||
| interval: Number(commitment.interval), | ||
| useDeliveryInbox: commitment.useDeliveryInbox, | ||
| walletAddress: commitment.walletAddress, | ||
| feeAmount: BigInt(commitment.feeAmount), | ||
| feeToken: commitment.feeToken, | ||
| verifier: commitment.verifier, | ||
| useDeliveryInbox: commitment.useDeliveryInbox, | ||
| coordinator: commitment.coordinator, | ||
| verifierFee: BigInt(commitment.verifierFee), |
There was a problem hiding this comment.
The parseCommitment method has been updated to reflect the new ABI structure, removing client and wallet and adding walletAddress, coordinator, and verifierFee. This is a critical change for data integrity and correct interaction with the updated contracts. Ensure that all parts of the system that rely on this parsed Commitment object are aware of these structural changes.
| return { | ||
| requestId: commitment.requestId, | ||
| subscriptionId: BigInt(commitment.subscriptionId), | ||
| interval: Number(commitment.interval), | ||
| redundancy: Number(commitment.redundancy), | ||
| containerId: commitment.containerId, | ||
| client: commitment.client, | ||
| wallet: commitment.wallet, | ||
| feeToken: commitment.feeToken, | ||
| interval: Number(commitment.interval), | ||
| useDeliveryInbox: commitment.useDeliveryInbox, | ||
| walletAddress: commitment.walletAddress, | ||
| feeAmount: BigInt(commitment.feeAmount), | ||
| feeToken: commitment.feeToken, | ||
| verifier: commitment.verifier, | ||
| useDeliveryInbox: commitment.useDeliveryInbox, | ||
| coordinator: commitment.coordinator, | ||
| verifierFee: BigInt(commitment.verifierFee), | ||
| }; |
There was a problem hiding this comment.
The parseCommitment method in RouterContract has been updated to align with the new ABI, removing client and wallet and adding walletAddress, coordinator, and verifierFee. This is crucial for maintaining data consistency and correct contract interactions. Verify that all consuming components correctly handle this updated structure.
1 participant
V2 remove redundancy & optimize gas usage