chore: refactor evm indexer #15
Conversation
Signed-off-by: tcar <[email protected]>
tcar121293
requested review from
mpetrunic,
mpetrun5,
MakMuftic and
mj52951
and removed request for
mj52951
| }) | ||
| .setBlockRange({ from: domain.startBlock }) | ||
| .setFinalityConfirmation(domain.blockConfirmations) | ||
| .setFields({ |
There was a problem hiding this comment.
This was my mistake. This .setFields should be removed as it is not exported anywhere.
There was a problem hiding this comment.
but isn't that used when using subsquid data lake to fetch only logs?
https://docs.sqd.dev/tron-indexing/tron-batch-processor/field-selection/#set-fields
Obviously when using rpc it's useless
There was a problem hiding this comment.
getProcessor function returns EvmBatchProcessor but if we want to export any of the fields it should return EvmBatchProcessor<definedFields>.
Also, log-topics is by default true. But that leads to a possible future optimization of setting other unused "default true" fields to "false" to improve performance (should be checked if there are any).
| this.provider = api; | ||
| } | ||
|
|
||
| public parseDestination(hexData: string, resourceType: ResourceType): string { |
There was a problem hiding this comment.
Is this called anywhere? Purpose of this is when an EVM deposit is made to a Substrate network, Substrate destination address is correctly parsed.
| } | ||
| } | ||
|
|
||
| private generateTransferID( |
There was a problem hiding this comment.
Not sure, but maybe put this as a helper function accessible to all parsers as this is universally how a transferID is generated for every network type (both EVM and Substrate).
| ): Promise<void>; | ||
| } | ||
|
|
||
| export class Indexer { |
There was a problem hiding this comment.
Will need to think about this whole file and the whole interface approach (maybe also test it out) because e.g. in Substrate the ctx type is different, and in blocks Log types are not fetched but Event types etc. So I think some change of the logic will be needed.
There was a problem hiding this comment.
I think if we define fields like Fields = EvmBatchProcessorFields<EvmBatchProcessor> | SubstrateBatchProcessorFields<SubstrateBatchProcessor> ; it should work?
| return evmProcessor; | ||
| } | ||
|
|
||
| public async processDeposits( |
There was a problem hiding this comment.
Also ran into this now. Not sure should processing deposits/executions/failedExec be domain-specific nor network-specific. Because while parsing we can store all data into same types and make processing just communication of the database with "the universal types".
|
|
||
| export const getSharedConfig = async (): Promise<SharedConfig> => { | ||
| try { | ||
| const sharedConfigURL = process.env.SHARED_CONFIG_URL!; |
There was a problem hiding this comment.
have you considered file where you will load all envs and validate them? Might be easier to maintain them later on.
| return (await response.json()) as SharedConfig; | ||
| } catch (e) { | ||
| logger.error(`Failed to fetch config for ${process.env.STAGE || ""}`, e); | ||
| return Promise.reject(e); |
There was a problem hiding this comment.
| return Promise.reject(e); | |
| throw e |
| ); | ||
| } | ||
| const resource = fromDomain.resources.find( | ||
| (resource) => resource.resourceId == event.resourceID, |
There was a problem hiding this comment.
should we do case insensitive comparison here?
| depositNonce: event.depositNonce, | ||
| txHash: transaction.hash, | ||
| message: ethers.decodeBytes32String( | ||
| "0x" + Buffer.from(event.lowLevelData).subarray(-64).toString(), |
There was a problem hiding this comment.
Maybe use https://nodejs.org/api/buffer.html#static-method-bufferfromarraybuffer-byteoffset-length constructor to avoid memory copy
| const token = this.getContract( | ||
| provider, | ||
| fee.tokenAddress, | ||
| ContractType.ERC20, | ||
| ); | ||
| tokenSymbol = (await token.symbol()) as string; | ||
| decimals = Number(await token.decimals()); |
There was a problem hiding this comment.
we should probably cache this as those are at least two rpc calls. Like can we get all sygma supported erc20 tokens and store their details?
There was a problem hiding this comment.
caching should be done on init
| }) | ||
| .setBlockRange({ from: domain.startBlock }) | ||
| .setFinalityConfirmation(domain.blockConfirmations) | ||
| .setFields({ |
There was a problem hiding this comment.
but isn't that used when using subsquid data lake to fetch only logs?
https://docs.sqd.dev/tron-indexing/tron-batch-processor/field-selection/#set-fields
Obviously when using rpc it's useless
| ResourceType, | ||
| Domain as DomainSDK, | ||
| Resource, | ||
| } from "@buildwithsygma/sygma-sdk-core"; |
There was a problem hiding this comment.
updated types can be used from @buildwithsygma/core, @buildwithsygma/evm and @buildwithsygma/substrate packages, this version of SDK is deprecated
| } | ||
| }; | ||
|
|
||
| export const getRpcMap = (): Map<number, string> => { |
There was a problem hiding this comment.
better to index RPC w.r.t caipId
| sharedConfig: SharedConfig, | ||
| rpcUrl: Map<number, string>, | ||
| ): Map<number, DomainConfig> { | ||
| const domainConfig = new Map<number, DomainConfig>(); |
| case ResourceType.PERMISSIONLESS_GENERIC: | ||
| { | ||
| // 32 + 2 + 1 + 1 + 20 + 20 | ||
| const lenExecuteFuncSignature = Number( |
There was a problem hiding this comment.
this logic can be extracted to a static function like decodeGenericCall
| const token = this.getContract( | ||
| provider, | ||
| fee.tokenAddress, | ||
| ContractType.ERC20, | ||
| ); | ||
| tokenSymbol = (await token.symbol()) as string; | ||
| decimals = Number(await token.decimals()); |
There was a problem hiding this comment.
caching should be done on init
No description provided.