improve(LineaFinalizer): Force RPC calls through custom provider #1931
Conversation
The functions that we use in the Linea SDK make event queries from block 0 to "latest" by default which means that some of our RPC's can't be used with the Finalizer. Additionally, we're missing out our custom provider retry and caching logic. This PR removes all instances of making RPC calls via the Linea SDK's provider and replace with making the same call through our custom Provider. The latest [Linea SDK](https://github.com/Consensys/linea-monorepo/blob/3fbe660683a318b6fa1b63ec518f948791536352/sdk/src/sdk/LineaSDK.ts#L56) code looks like it'll support injecting custom providers but its not released yet on NPM. So this code should be removed eventually once a later SDK version is released.
nicholaspai
requested review from
mrice32,
pxrl and
james-a-morris
as code owners
| return new Contract( | ||
| l1ClaimingService.l2Contract.contract.address, | ||
| l1ClaimingService.l2Contract.contract.interface, | ||
| l2Provider | ||
| ); |
There was a problem hiding this comment.
Could this alternatively work?
| return new Contract( | |
| l1ClaimingService.l2Contract.contract.address, | |
| l1ClaimingService.l2Contract.contract.interface, | |
| l2Provider | |
| ); | |
| return l1ClaimingService.l2Contract.contract.connect(l2Provider); |
| export function getL1MessageServiceContractFromL1ClaimingService( | ||
| l1ClaimingService: L1ClaimingService, | ||
| l1Provider: ethers.providers.Provider | ||
| ): Contract { | ||
| return new Contract( | ||
| l1ClaimingService.l1Contract.contract.address, | ||
| l1ClaimingService.l1Contract.contract.interface, | ||
| l1Provider | ||
| ); | ||
| } |
| // Normally we avoid importing directly from a node_modules' /dist package but we need access to some | ||
| // of the internal classes and functions in order to replicate SDK logic so that we can by pass hardcoded | ||
| // ethers.Provider instances and use our own custom provider instead. | ||
| import { L2MessageServiceContract } from "@consensys/linea-sdk/dist/lib/contracts"; |
There was a problem hiding this comment.
wdyt about exporting these from a new file, so we can contain it and only have to document it there? i.e. src/finalizer/utils/linea/import.ts or something.
| messageHash: string, | ||
| l2Provider: ethers.providers.Provider | ||
| ): Promise<OnChainMessageStatus> { | ||
| const l2Contract = new Contract(messageService.contract.address, messageService.contract.interface, l2Provider); |
There was a problem hiding this comment.
Also wondering if you can just connect the provider to the existing contract, but not sure whether it might be some weird type.
| const l2Contract = new Contract(messageService.contract.address, messageService.contract.interface, l2Provider); | |
| const l2Contract = messageService.contract.connect(l2Provider); |
I haven't been able to time it well. I think one way to do it is i can temporarily pause the production finalizer for linea and run this locally. |
There was a problem hiding this comment.
This makes sense but I think we should avoid accessing the Linea package in an unintended way. We've forked/patched packages before so that they play nicely with our imports - this could be a viable workaround.
Otherwise there's nothing that stands out - I'll approve pending my comments
| @@ -209,6 +210,7 @@ export async function arbStackFinalizer( | |||
| logger.debug({ | |||
| at: `Finalizer#${networkName}Finalizer`, | |||
| message: `Withdrawal event for ${amountFromWei} of ${l1TokenInfo.symbol} is too recent to finalize`, | |||
| timeUntilFinalization: `${((event.blockNumber - latestBlockToFinalize) * l2BlockTime) / 60 / 60}`, | |||
There was a problem hiding this comment.
Did we want to floor this? Could be good for readability to at least toFixed() this output to keep the logs clean
| import { L1MessageServiceContract, L2MessageServiceContract } from "@consensys/linea-sdk/dist/lib/contracts"; | ||
| import { L1ClaimingService } from "@consensys/linea-sdk/dist/lib/sdk/claiming/L1ClaimingService"; | ||
| import { MessageSentEvent } from "@consensys/linea-sdk/dist/typechain/L2MessageService"; |
There was a problem hiding this comment.
Is this the only location we need these imports? Could we make a patchfile to have these vars be exported?
The functions that we use in the Linea SDK make event queries from block 0 to "latest" by default which means that some of our RPC's can't be used with the Finalizer. Additionally, we're missing out our custom provider retry and caching logic.
This PR removes all instances of making RPC calls via the Linea SDK's provider and replace with making the same call through our custom Provider.
The latest Linea SDK code looks like it'll support injecting custom providers but its not released yet on NPM. So this code should be removed eventually once a later SDK version is released.