feat: batches info l1 metric #41
Conversation
ZKS-74 Batches Info
Use DiamodProxy address: 0x32400084c286cf3e17e7b677ea9583e60a000324 AC:
|
| if (!Number.isInteger(chainId)) { | ||
| throw new InvalidChainId("chain id must be an integer"); | ||
| } | ||
| const chainIdBn = BigInt(chainId); | ||
| let diamondProxyAddress: Address | undefined = this.diamondContracts.get(chainId); | ||
|
|
||
| if (!diamondProxyAddress) { | ||
| diamondProxyAddress = await this.evmProviderService.readContract( | ||
| this.bridgeHub.address, | ||
| this.bridgeHub.abi, | ||
| "getHyperchain", | ||
| [chainIdBn], | ||
| ); | ||
| this.diamondContracts.set(chainId, diamondProxyAddress); | ||
| } |
There was a problem hiding this comment.
extracted this logic in the following pr cause i noticed that we need it for chainType and probably others in the future :)
There was a problem hiding this comment.
should we handle the case where address returned is the zeroAddress? (this would be the case where chainId is a non-existent chain)
There was a problem hiding this comment.
yea, good catch, i will check how is this function implemented, if reverts or returns Address(0)
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| */ | ||
| async getBatchesInfo(chainId: number): Promise<BatchesInfo> { | ||
| if (!Number.isInteger(chainId)) { | ||
| throw new InvalidChainId("chain id must be an integer"); |
| if (!Number.isInteger(chainId)) { | ||
| throw new InvalidChainId("chain id must be an integer"); | ||
| } | ||
| const chainIdBn = BigInt(chainId); | ||
| let diamondProxyAddress: Address | undefined = this.diamondContracts.get(chainId); | ||
|
|
||
| if (!diamondProxyAddress) { | ||
| diamondProxyAddress = await this.evmProviderService.readContract( | ||
| this.bridgeHub.address, | ||
| this.bridgeHub.abi, | ||
| "getHyperchain", | ||
| [chainIdBn], | ||
| ); | ||
| this.diamondContracts.set(chainId, diamondProxyAddress); | ||
| } |
There was a problem hiding this comment.
should we handle the case where address returned is the zeroAddress? (this would be the case where chainId is a non-existent chain)
| super(message); | ||
| this.name = "L1MetricsServiceException"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Are we ok with two errors in one single file? I'm fine with both single error per file and multi error files, just asking in case you forgot to put the L1MetricsServiceException in its own file
| throw new InvalidChainId("chain id must be an integer"); | ||
| } | ||
| const chainIdBn = BigInt(chainId); | ||
| let diamondProxyAddress: Address | undefined = this.diamondContracts.get(chainId); |
There was a problem hiding this comment.
This is unrelated to the PR but you got me wondering, why is the type of the diamondContracts keys (ie ChainId) a number? Any tech/business specific reason to not use bigints?
There was a problem hiding this comment.
i thought there was no need for using bigints for chainId since they are not gona be huge numbers, but probably its better to keep types consistent with returned values from blockchain, i will change it
There was a problem hiding this comment.
@0xkenj1 we can add this revisions to tech debt because i had the same thought as you, maybe the chainId argument can be a bigint directly too 🤔? (then we can remove the isInteger validation)
There was a problem hiding this comment.
yep, we wont need that validation, i will fix this one here but will added it to linear to tackle it before moving to api integration
🤖 Linear
Closes ZKS-74
Description
getBatchesInfologic onL1MetricsService