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

Refactor: Add readMulticall route for batch contract reads #773

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

d4mr
Copy link
Member

@d4mr d4mr commented Nov 15, 2024

Test with example curl:

curl --request POST \
  --url http://localhost:3005/contract/421614/read-batch \
  --header 'Authorization: Bearer {{authToken}}' \
  --header 'content-type: application/json' \
  --data '{
  "calls": [
    {
      "contractAddress": "0x75faf114eafb1BDbe2F0316DF893fd58CE46AA4d",
      "functionName": "name"
    },
    {
      "contractAddress": "0x75faf114eafb1BDbe2F0316DF893fd58CE46AA4d",
      "functionName": "totalSupply"
    }
  ]
}'

PR-Codex overview

This PR primarily focuses on adding support for batch reading multiple contract calls using Multicall3 in the src/server/routes/contract/read/read-batch.ts file. It also updates various dependencies to their latest versions.

Detailed summary

  • Updated thirdweb dependency from 5.61.3 to 5.69.0.
  • Added readMulticall function to handle batch read operations in src/server/routes/contract/read/read-batch.ts.
  • Registered readMulticall in src/server/routes/index.ts.
  • Updated various dependencies in package.json and yarn.lock.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@d4mr
Copy link
Member Author

d4mr commented Nov 15, 2024

changelog

@d4mr d4mr requested a review from arcoraven November 21, 2024 13:54
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

🚮 Removed packages: npm/[email protected]

View full report↗︎

// Encode each read call
const encodedCalls = await Promise.all(
calls.map(async (call) => {
const contract = await getContract({

Choose a reason for hiding this comment

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

You don't have to await getContract btw

Reply: Static<typeof responseSchema>;
};

export async function readMulticall(fastify: FastifyInstance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit and non-blocking: I started calling route names readMulticallRoute. The reason is we have so many exported functions that end up having the same name (for example: createAccessToken might be the route, or the cached call, or the raw db call, who knows?)

So having that suffix is nice for clarity.


const readMulticallRequestSchema = Type.Object({
calls: Type.Array(readCallRequestItemSchema),
multicallAddress: Type.Optional(Type.String()),
Copy link
Contributor

@arcoraven arcoraven Nov 23, 2024

Choose a reason for hiding this comment

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

Add a description so people know they don't need to set this:

Override a specific multicall contract. Defaults to Multicall3.

schema: {
summary: "Batch read from multiple contracts",
description:
"Execute multiple contract read operations in a single call using Multicall3",
Copy link
Contributor

@arcoraven arcoraven Nov 23, 2024

Choose a reason for hiding this comment

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

IMO don't need to mention multicall3 as it's a low level implementation detail. You can refer to "using multicall" since that's a web3 concept.

value: 0n,
});

const calldata = await resolvePromisedValue(transaction.data);
Copy link
Contributor

@arcoraven arcoraven Nov 23, 2024

Choose a reason for hiding this comment

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

Don't use resolvePromisedValue. I should add a @deprecated tag on the function. Use this instead:

import { encode } from "thirdweb";
const callData = await encode(transaction);

Comment on lines +100 to +103
// stubbing gas values so that the call can be encoded
maxFeePerGas: 30n,
maxPriorityFeePerGas: 1n,
value: 0n,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these need to be stubbed? What happens if we leave them undefined?


return {
success,
result: success ? bigNumberReplacer(decoded) : null,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not need bigNumberReplacer. We're on v5 SDK which does not rely on ethers.

description:
"Execute multiple contract read operations in a single call using Multicall3",
tags: ["Contract"],
operationId: "readMulticall",
Copy link
Contributor

Choose a reason for hiding this comment

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

readBatch is preferred since "multicall" is the underlying approach but the use case is a batch read.

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.

3 participants