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

Add an example test case for utilizing the Multicall3 contract #369

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Jul 19, 2024

Description

Multicall3: https://github.com/mds1/multicall


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Summary by CodeRabbit

  • New Features

    • Introduced a Multicall3 smart contract for batch processing of multiple Ethereum function calls, improving transaction efficiency and reducing gas costs.
    • Added new end-to-end tests for validating deployment and interaction of the Multicall3 contract and a storage contract.
  • Bug Fixes

    • Enhanced error handling within the Multicall3 contract to preserve transaction integrity when individual calls fail.
  • Documentation

    • Added a new ABI definition for the Multicall3 contract, detailing its capabilities and functions.

@m-Peter m-Peter added this to the Flow-EVM-M2 milestone Jul 19, 2024
@m-Peter m-Peter self-assigned this Jul 19, 2024
Copy link
Contributor

coderabbitai bot commented Jul 19, 2024

Walkthrough

This update introduces a new Multicall3 smart contract, enhancing the ability to batch multiple function calls into a single transaction, optimizing gas usage. It includes comprehensive tests for deploying and interacting with the contract using Web3.js. The changes improve the end-to-end test suite and expand functionality for efficient smart contract interactions, particularly in decentralized finance applications.

Changes

File Path Change Summary
tests/e2e_web3js_test.go Adds a test for deploying and interacting with the multicall3 contract, improving end-to-end test coverage for Web3 interactions.
tests/fixtures/multicall3.byte Introduces smart contract bytecode for the multicall3 functionality, allowing batching of multiple function calls to optimize gas usage and transaction efficiency.
tests/fixtures/multicall3.sol Introduces the Multicall3 smart contract with methods for aggregating calls, handling errors, and managing input/output efficiently, enhancing previous versions.
tests/fixtures/multicall3ABI.json Defines the ABI for the Multicall3 contract, specifying functions that enable batch processing of calls and retrieval of blockchain data, streamlining interactions.
tests/web3js/eth_multicall3_contract_test.js Adds unit tests for deploying the multicall3 contract and validating interactions with a storage contract, ensuring correct operation of the multicall functionality.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Web3
    participant Multicall3
    participant Storage

    User->>Web3: Deploy Storage Contract
    Web3->>Storage: Deploy
    Storage-->>Web3: Contract Address
    
    User->>Web3: Deploy Multicall3 Contract
    Web3->>Multicall3: Deploy
    Multicall3-->>Web3: Contract Address
    
    User->>Web3: Call aggregate3 on Multicall3
    Web3->>Multicall3: Execute Calls
    Multicall3->>Storage: Call sum(10, 10)
    Storage-->>Multicall3: Return 20
    Multicall3->>Storage: Call sum(10, 40)
    Storage-->>Multicall3: Return 50
    
    Multicall3-->>Web3: Return Results
    Web3-->>User: Display Results
Loading

🐇 In code and contracts, we now play,
With Multicall3, we save the day!
Batch our calls, how sweet the sound,
Efficiency and gas, all around!
Hop along with tests that shine,
In the world of Web3, we redefine! 🌟


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.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration 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: 10

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d37bd82 and d495433.

Files selected for processing (5)
  • tests/e2e_web3js_test.go (1 hunks)
  • tests/fixtures/multicall3.byte (1 hunks)
  • tests/fixtures/multicall3.sol (1 hunks)
  • tests/fixtures/multicall3ABI.json (1 hunks)
  • tests/web3js/eth_multicall3_contract_test.js (1 hunks)
Additional comments not posted (21)
tests/web3js/eth_multicall3_contract_test.js (3)

1-4: Import statements look good.

The required modules are imported correctly.


12-14: Verify the initial value retrieval logic.

The retrieval of the initial value should include decoding the result to ensure it matches the expected value.

- result = await web3.eth.call({ to: contractAddress, data: callRetrieve }, "latest")
+ result = await web3.eth.call({ to: contractAddress, data: callRetrieve }, "latest");
+ let decodedResult = web3.eth.abi.decodeParameter('uint256', result);
+ assert.equal(decodedResult, initValue);

48-56: Verify the decoding of the result.

The decoding of the result should be verified to ensure it matches the expected structure.

- let decodedResult = web3.eth.abi.decodeParameter(
-     {
-         'Result[]': {
-             'success': 'bool',
-             'returnData': 'bytes'
-         }
-     },
-     result
- )
+ let decodedResult;
+ try {
+   decodedResult = web3.eth.abi.decodeParameter(
+       {
+           'Result[]': {
+               'success': 'bool',
+               'returnData': 'bytes'
+           }
+       },
+       result
+   );
+ } catch (error) {
+   assert.fail(`Decoding result failed: ${error.message}`);
+ }
tests/e2e_web3js_test.go (1)

33-35: New test case addition looks good.

The new test case for deploying and interacting with the Multicall3 contract is correctly added.

tests/fixtures/multicall3.sol (1)

98-123: Ensure proper error handling for the aggregate3 function.

The aggregate3 function should include error handling for the call results.

- for (uint256 i = 0; i < length;) {
-     Result memory result = returnData[i];
-     calli = calls[i];
-     (result.success, result.returnData) = calli.target.call(calli.callData);
-     assembly {
-         if iszero(or(calldataload(add(calli, 0x20)), mload(result))) {
-             mstore(0x00, 0x08c379a000000000000000000000000000000000000000000000000000000000)
-             mstore(0x04, 0x0000000000000000000000000000000000000000000000000000000000000020)
-             mstore(0x24, 0x0000000000000000000000000000000000000000000000000000000000000017)
-             mstore(0x44, 0x4d756c746963616c6c333a2063616c6c206661696c6564000000000000000000)
-             revert(0x00, 0x64)
-         }
-     }
-     unchecked { ++i; }
- }
+ for (uint256 i = 0; i < length;) {
+     Result memory result = returnData[i];
+     calli = calls[i];
+     (result.success, result.returnData) = calli.target.call(calli.callData);
+     assembly {
+         if iszero(or(calldataload(add(calli, 0x20)), mload(result))) {
+             mstore(0x00, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+             mstore(0x04, 0x0000000000000000000000000000000000000000000000000000000000000020)
+             mstore(0x24, 0x0000000000000000000000000000000000000000000000000000000000000017)
+             mstore(0x44, 0x4d756c746963616c6c333a2063616c6c206661696c6564000000000000000000)
+             revert(0x00, 0x64)
+         }
+     }
+     unchecked { ++i; }
+ }

Likely invalid or redundant comment.

tests/fixtures/multicall3ABI.json (16)

2-37: LGTM!

The aggregate function definition appears correct. The input and output types are appropriate for the described functionality.


38-85: LGTM!

The aggregate3 function definition appears correct. The input and output types are appropriate for the described functionality.


86-138: LGTM!

The aggregate3Value function definition appears correct. The input and output types are appropriate for the described functionality.


139-191: LGTM!

The blockAndAggregate function definition appears correct. The input and output types are appropriate for the described functionality.


192-204: LGTM!

The getBasefee function definition appears correct. The output type is appropriate for the described functionality.


205-223: LGTM!

The getBlockHash function definition appears correct. The input and output types are appropriate for the described functionality.


224-236: LGTM!

The getBlockNumber function definition appears correct. The output type is appropriate for the described functionality.


237-249: LGTM!

The getChainId function definition appears correct. The output type is appropriate for the described functionality.


250-262: LGTM!

The getCurrentBlockCoinbase function definition appears correct. The output type is appropriate for the described functionality.


263-275: LGTM!

The getCurrentBlockDifficulty function definition appears correct. The output type is appropriate for the described functionality.


276-288: LGTM!

The getCurrentBlockGasLimit function definition appears correct. The output type is appropriate for the described functionality.


289-301: LGTM!

The getCurrentBlockTimestamp function definition appears correct. The output type is appropriate for the described functionality.


302-320: LGTM!

The getEthBalance function definition appears correct. The input and output types are appropriate for the described functionality.


321-333: LGTM!

The getLastBlockHash function definition appears correct. The output type is appropriate for the described functionality.


334-381: LGTM!

The tryAggregate function definition appears correct. The input and output types are appropriate for the described functionality.


382-439: LGTM!

The tryBlockAndAggregate function definition appears correct. The input and output types are appropriate for the described functionality.

Comment on lines +41 to +47
result = await web3.eth.call(
{
to: multicall3Address,
data: callAggregate3
},
"latest"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the multicall3 aggregate call.

The aggregate call should include error handling to catch any issues during the call.

- result = await web3.eth.call(
-     {
-         to: multicall3Address,
-         data: callAggregate3
-     },
-     "latest"
- )
+ try {
+   result = await web3.eth.call(
+       {
+           to: multicall3Address,
+           data: callAggregate3
+       },
+       "latest"
+   );
+ } catch (error) {
+   assert.fail(`Multicall3 aggregate call failed: ${error.message}`);
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
result = await web3.eth.call(
{
to: multicall3Address,
data: callAggregate3
},
"latest"
)
try {
result = await web3.eth.call(
{
to: multicall3Address,
data: callAggregate3
},
"latest"
);
} catch (error) {
assert.fail(`Multicall3 aggregate call failed: ${error.message}`);
}

Comment on lines +16 to +18
let multicall3 = await helpers.deployContract("multicall3")
let multicall3Address = multicall3.receipt.contractAddress

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for Multicall3 contract deployment.

The deployment of the Multicall3 contract should include error handling to catch any issues during deployment.

- let multicall3 = await helpers.deployContract("multicall3")
+ let multicall3;
+ try {
+   multicall3 = await helpers.deployContract("multicall3");
+ } catch (error) {
+   assert.fail(`Multicall3 contract deployment failed: ${error.message}`);
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let multicall3 = await helpers.deployContract("multicall3")
let multicall3Address = multicall3.receipt.contractAddress
let multicall3;
try {
multicall3 = await helpers.deployContract("multicall3");
} catch (error) {
assert.fail(`Multicall3 contract deployment failed: ${error.message}`);
}
let multicall3Address = multicall3.receipt.contractAddress

Comment on lines +58 to +64
assert.lengthOf(decodedResult, 2)

assert.equal(decodedResult[0].success, true)
assert.equal(decodedResult[0].returnData, 20n)

assert.equal(decodedResult[1].success, true)
assert.equal(decodedResult[1].returnData, 50n)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper assertions for the decoded results.

The assertions for the decoded results should include checks for the length and values of the results.

- assert.lengthOf(decodedResult, 2)
- assert.equal(decodedResult[0].success, true)
- assert.equal(decodedResult[0].returnData, 20n)
- assert.equal(decodedResult[1].success, true)
- assert.equal(decodedResult[1].returnData, 50n)
+ assert.lengthOf(decodedResult, 2, 'Decoded result length mismatch');
+ assert.equal(decodedResult[0].success, true, 'First call success mismatch');
+ assert.equal(BigInt(decodedResult[0].returnData), 20n, 'First call return data mismatch');
+ assert.equal(decodedResult[1].success, true, 'Second call success mismatch');
+ assert.equal(BigInt(decodedResult[1].returnData), 50n, 'Second call return data mismatch');
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assert.lengthOf(decodedResult, 2)
assert.equal(decodedResult[0].success, true)
assert.equal(decodedResult[0].returnData, 20n)
assert.equal(decodedResult[1].success, true)
assert.equal(decodedResult[1].returnData, 50n)
assert.lengthOf(decodedResult, 2, 'Decoded result length mismatch');
assert.equal(decodedResult[0].success, true, 'First call success mismatch');
assert.equal(BigInt(decodedResult[0].returnData), 20n, 'First call return data mismatch');
assert.equal(decodedResult[1].success, true, 'Second call success mismatch');
assert.equal(BigInt(decodedResult[1].returnData), 50n, 'Second call return data mismatch');

Comment on lines +24 to +39
let callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI()
let callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI()
let callAggregate3 = await multicall3.contract.methods.aggregate3(
[
{
target: contractAddress,
allowFailure: false,
callData: callSum20
},
{
target: contractAddress,
allowFailure: false,
callData: callSum50
}
]
).encodeABI()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for method calls.

The method calls should include error handling to catch any issues during the calls.

- let callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI()
- let callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI()
- let callAggregate3 = await multicall3.contract.methods.aggregate3(
-     [
-         {
-             target: contractAddress,
-             allowFailure: false,
-             callData: callSum20
-         },
-         {
-             target: contractAddress,
-             allowFailure: false,
-             callData: callSum50
-         }
-     ]
- ).encodeABI()
+ let callSum20, callSum50, callAggregate3;
+ try {
+   callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI();
+   callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI();
+   callAggregate3 = await multicall3.contract.methods.aggregate3(
+       [
+           {
+               target: contractAddress,
+               allowFailure: false,
+               callData: callSum20
+           },
+           {
+               target: contractAddress,
+               allowFailure: false,
+               callData: callSum50
+           }
+       ]
+   ).encodeABI();
+ } catch (error) {
+   assert.fail(`Method call failed: ${error.message}`);
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI()
let callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI()
let callAggregate3 = await multicall3.contract.methods.aggregate3(
[
{
target: contractAddress,
allowFailure: false,
callData: callSum20
},
{
target: contractAddress,
allowFailure: false,
callData: callSum50
}
]
).encodeABI()
let callSum20, callSum50, callAggregate3;
try {
callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI();
callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI();
callAggregate3 = await multicall3.contract.methods.aggregate3(
[
{
target: contractAddress,
allowFailure: false,
callData: callSum20
},
{
target: contractAddress,
allowFailure: false,
callData: callSum50
}
]
).encodeABI();
} catch (error) {
assert.fail(`Method call failed: ${error.message}`);
}

Comment on lines +6 to +8
it('deploy contract and interact', async () => {
let deployed = await helpers.deployContract("storage")
let contractAddress = deployed.receipt.contractAddress
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for contract deployment.

The deployment of the contract should include error handling to catch any issues during deployment.

- let deployed = await helpers.deployContract("storage")
+ let deployed;
+ try {
+   deployed = await helpers.deployContract("storage");
+ } catch (error) {
+   assert.fail(`Contract deployment failed: ${error.message}`);
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('deploy contract and interact', async () => {
let deployed = await helpers.deployContract("storage")
let contractAddress = deployed.receipt.contractAddress
it('deploy contract and interact', async () => {
let deployed;
try {
deployed = await helpers.deployContract("storage");
} catch (error) {
assert.fail(`Contract deployment failed: ${error.message}`);
}
let contractAddress = deployed.receipt.contractAddress

Comment on lines +129 to +161
function aggregate3Value(Call3Value[] calldata calls) public payable returns (Result[] memory returnData) {
uint256 valAccumulator;
uint256 length = calls.length;
returnData = new Result[](length);
Call3Value calldata calli;
for (uint256 i = 0; i < length;) {
Result memory result = returnData[i];
calli = calls[i];
uint256 val = calli.value;
// Humanity will be a Type V Kardashev Civilization before this overflows - andreas
// ~ 10^25 Wei in existence << ~ 10^76 size uint fits in a uint256
unchecked { valAccumulator += val; }
(result.success, result.returnData) = calli.target.call{value: val}(calli.callData);
assembly {
// Revert if the call fails and failure is not allowed
// `allowFailure := calldataload(add(calli, 0x20))` and `success := mload(result)`
if iszero(or(calldataload(add(calli, 0x20)), mload(result))) {
// set "Error(string)" signature: bytes32(bytes4(keccak256("Error(string)")))
mstore(0x00, 0x08c379a000000000000000000000000000000000000000000000000000000000)
// set data offset
mstore(0x04, 0x0000000000000000000000000000000000000000000000000000000000000020)
// set length of revert string
mstore(0x24, 0x0000000000000000000000000000000000000000000000000000000000000017)
// set revert string: bytes32(abi.encodePacked("Multicall3: call failed"))
mstore(0x44, 0x4d756c746963616c6c333a2063616c6c206661696c6564000000000000000000)
revert(0x00, 0x84)
}
}
unchecked { ++i; }
}
// Finally, make sure the msg.value = SUM(call[0...i].value)
require(msg.value == valAccumulator, "Multicall3: value mismatch");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the aggregate3Value function.

The aggregate3Value function should include error handling for the call results.

- for (uint256 i = 0; i < length;) {
-     Result memory result = returnData[i];
-     calli = calls[i];
-     uint256 val = calli.value;
-     unchecked { valAccumulator += val; }
-     (result.success, result.returnData) = calli.target.call{value: val}(calli.callData);
-     assembly {
-         if iszero(or(calldataload(add(calli, 0x20)), mload(result))) {
-             mstore(0x00, 0x08c379a000000000000000000000000000000000000000000000000000000000)
-             mstore(0x04, 0x0000000000000000000000000000000000000000000000000000000000000020)
-             mstore(0x24, 0x0000000000000000000000000000000000000000000000000000000000000017)
-             mstore(0x44, 0x4d756c746963616c6c333a2063616c6c206661696c6564000000000000000000)
-             revert(0x00, 0x84)
-         }
-     }
-     unchecked { ++i; }
- }
- require(msg.value == valAccumulator, "Multicall3: value mismatch");
+ for (uint256 i = 0; i < length;) {
+     Result memory result = returnData[i];
+     calli = calls[i];
+     uint256 val = calli.value;
+     unchecked { valAccumulator += val; }
+     (result.success, result.returnData) = calli.target.call{value: val}(calli.callData);
+     assembly {
+         if iszero(or(calldataload(add(calli, 0x20)), mload(result))) {
+             mstore(0x00, 0x08c379a000000000000000000000000000000

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +79 to +83
function tryBlockAndAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
blockNumber = block.number;
blockHash = blockhash(block.number);
returnData = tryAggregate(requireSuccess, calls);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the tryBlockAndAggregate function.

The tryBlockAndAggregate function should include error handling for the call results.

- returnData = tryAggregate(requireSuccess, calls);
+ try {
+     returnData = tryAggregate(requireSuccess, calls);
+ } catch (error) {
+     revert("Multicall3: tryBlockAndAggregate failed");
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function tryBlockAndAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
blockNumber = block.number;
blockHash = blockhash(block.number);
returnData = tryAggregate(requireSuccess, calls);
}
function tryBlockAndAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
blockNumber = block.number;
blockHash = blockhash(block.number);
try {
returnData = tryAggregate(requireSuccess, calls);
} catch (error) {
revert("Multicall3: tryBlockAndAggregate failed");
}
}

Comment on lines +91 to +93
function blockAndAggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
(blockNumber, blockHash, returnData) = tryBlockAndAggregate(true, calls);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the blockAndAggregate function.

The blockAndAggregate function should include error handling for the call results.

- (blockNumber, blockHash, returnData) = tryBlockAndAggregate(true, calls);
+ try {
+     (blockNumber, blockHash, returnData) = tryBlockAndAggregate(true, calls);
+ } catch (error) {
+     revert("Multicall3: blockAndAggregate failed");
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function blockAndAggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
(blockNumber, blockHash, returnData) = tryBlockAndAggregate(true, calls);
}
function blockAndAggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes32 blockHash, Result[] memory returnData) {
try {
(blockNumber, blockHash, returnData) = tryBlockAndAggregate(true, calls);
} catch (error) {
revert("Multicall3: blockAndAggregate failed");
}
}

Comment on lines +37 to +53
/// @notice Backwards-compatible call aggregation with Multicall
/// @param calls An array of Call structs
/// @return blockNumber The block number where the calls were executed
/// @return returnData An array of bytes containing the responses
function aggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes[] memory returnData) {
blockNumber = block.number;
uint256 length = calls.length;
returnData = new bytes[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
bool success;
call = calls[i];
(success, returnData[i]) = call.target.call(call.callData);
require(success, "Multicall3: call failed");
unchecked { ++i; }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the aggregate function.

The aggregate function should include error handling for the call results.

- for (uint256 i = 0; i < length;) {
-     bool success;
-     call = calls[i];
-     (success, returnData[i]) = call.target.call(call.callData);
-     require(success, "Multicall3: call failed");
-     unchecked { ++i; }
- }
+ for (uint256 i = 0; i < length;) {
+     bool success;
+     call = calls[i];
+     (success, returnData[i]) = call.target.call(call.callData);
+     if (!success) {
+         revert("Multicall3: call failed");
+     }
+     unchecked { ++i; }
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// @notice Backwards-compatible call aggregation with Multicall
/// @param calls An array of Call structs
/// @return blockNumber The block number where the calls were executed
/// @return returnData An array of bytes containing the responses
function aggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes[] memory returnData) {
blockNumber = block.number;
uint256 length = calls.length;
returnData = new bytes[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
bool success;
call = calls[i];
(success, returnData[i]) = call.target.call(call.callData);
require(success, "Multicall3: call failed");
unchecked { ++i; }
}
}
/// @notice Backwards-compatible call aggregation with Multicall
/// @param calls An array of Call structs
/// @return blockNumber The block number where the calls were executed
/// @return returnData An array of bytes containing the responses
function aggregate(Call[] calldata calls) public payable returns (uint256 blockNumber, bytes[] memory returnData) {
blockNumber = block.number;
uint256 length = calls.length;
returnData = new bytes[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
bool success;
call = calls[i];
(success, returnData[i]) = call.target.call(call.callData);
if (!success) {
revert("Multicall3: call failed");
}
unchecked { ++i; }
}
}

Comment on lines +55 to +71
/// @notice Backwards-compatible with Multicall2
/// @notice Aggregate calls without requiring success
/// @param requireSuccess If true, require all calls to succeed
/// @param calls An array of Call structs
/// @return returnData An array of Result structs
function tryAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (Result[] memory returnData) {
uint256 length = calls.length;
returnData = new Result[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
Result memory result = returnData[i];
call = calls[i];
(result.success, result.returnData) = call.target.call(call.callData);
if (requireSuccess) require(result.success, "Multicall3: call failed");
unchecked { ++i; }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for the tryAggregate function.

The tryAggregate function should include error handling for the call results.

- for (uint256 i = 0; i < length;) {
-     Result memory result = returnData[i];
-     call = calls[i];
-     (result.success, result.returnData) = call.target.call(call.callData);
-     if (requireSuccess) require(result.success, "Multicall3: call failed");
-     unchecked { ++i; }
- }
+ for (uint256 i = 0; i < length;) {
+     Result memory result = returnData[i];
+     call = calls[i];
+     (result.success, result.returnData) = call.target.call(call.callData);
+     if (requireSuccess && !result.success) {
+         revert("Multicall3: call failed");
+     }
+     unchecked { ++i; }
+ }
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// @notice Backwards-compatible with Multicall2
/// @notice Aggregate calls without requiring success
/// @param requireSuccess If true, require all calls to succeed
/// @param calls An array of Call structs
/// @return returnData An array of Result structs
function tryAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (Result[] memory returnData) {
uint256 length = calls.length;
returnData = new Result[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
Result memory result = returnData[i];
call = calls[i];
(result.success, result.returnData) = call.target.call(call.callData);
if (requireSuccess) require(result.success, "Multicall3: call failed");
unchecked { ++i; }
}
}
/// @notice Backwards-compatible with Multicall2
/// @notice Aggregate calls without requiring success
/// @param requireSuccess If true, require all calls to succeed
/// @param calls An array of Call structs
/// @return returnData An array of Result structs
function tryAggregate(bool requireSuccess, Call[] calldata calls) public payable returns (Result[] memory returnData) {
uint256 length = calls.length;
returnData = new Result[](length);
Call calldata call;
for (uint256 i = 0; i < length;) {
Result memory result = returnData[i];
call = calls[i];
(result.success, result.returnData) = call.target.call(call.callData);
if (requireSuccess && !result.success) {
revert("Multicall3: call failed");
}
unchecked { ++i; }
}
}

Copy link
Contributor

@sideninja sideninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@sideninja sideninja merged commit 1ee807f into main Jul 19, 2024
2 checks passed
@m-Peter m-Peter deleted the multicall3-contract-test branch July 29, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants