alerting-forta icon indicating copy to clipboard operation
alerting-forta copied to clipboard

Using `blockNumber - 1` instead of `block.parentHash` is a subject to reorg false alarms

Open TheDZhon opened this issue 10 months ago • 4 comments

tl;dr

When the alert logic requires to query something from the previous block, it's better to use the block.parentHash instead of blockNumber - 1 to avoid issues with the Ethereum chain reorganizations.

How to fix

See the following reference code: https://github.com/lidofinance/alerting-forta/blob/main/ethereum-steth/src/clients/eth_provider.ts#L181-L196

  public async getBalanceByBlockHash(address: string, blockHash: string): Promise<E.Either<Error, BigNumber>> {
    try {
      const out = await retryAsync<EtherBigNumber>(
        async (): Promise<EtherBigNumber> => {
          return await this.jsonRpcProvider.getBalance(address, {
            blockHash: blockHash,
          } as never)
        },
        { delay: DELAY_IN_500MS, maxTry: ATTEMPTS_5 },
      )

      return E.right(new BigNumber(String(out)))
    } catch (e) {
      return E.left(new NetworkError(e, `Could not fetch balance by address ${address} and blockHash ${blockHash}`))
    }
  }

https://github.com/lidofinance/alerting-forta/blob/main/ethereum-steth/src/services/vault/Vault.srv.ts#L60-L63

const prevBlockWithdrawalVaultBalance = await this.ethProvider.getBalanceByBlockHash(
  this.withdrawalsVaultAddress,
  blockEvent.block.parentHash,
)

Affected scope

List of the files that affected by the issue:

grep --include=\*.ts -rnw '.' -e ".*umber - 1" --exclude-dir=node_modules

./ethereum-validators-set/src/subagents/lido-report/agent-lido-report.ts:841:    blockTag: txEvent.blockNumber - 1,
./ethereum-validators-set/src/subagents/exitbus-oracle/agent-exitbus-oracle.ts:283:      blockEvent.blockNumber - 1,
./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:140:      blockEvent.blockNumber - 1,
./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:148:        blockEvent.blockNumber - 1,
./l2-bridge-linea/src/clients/linea_block_client.ts:72:      const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
./l2-bridge-mantle/src/clients/mantle_block_client.ts:72:      const blocks = await this.mantleProvider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
./voting-watcher/src/agent-voting-watcher.ts:156:  const prevBlock = await ethersProvider.getBlock(blockEvent.blockNumber - 1);
./ethereum-steth/src/services/steth_operation/StethOperation.srv.ts:230:      this.ethProvider.getBufferedEther(shiftedBlockNumber - 1),
./l2-bridge-base/src/services/monitor_withdrawals.ts:54:    const withdrawalEvents = await this.withdrawalsClient.getWithdrawalEvents(pastl2Block, l2BlockNumber - 1)
./l2-bridge-base/src/clients/base_block_client.ts:72:      const blocks = await this.provider.fetchL2Blocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
./ethereum-financial/src/services/aave/Aave.srv.ts:63:        this.ethProvider.getStethBalance(this.aaveAstethAddress, blockEvent.number - 1),
./ethereum-financial/src/services/aave/Aave.srv.ts:64:        await this.ethProvider.getTotalSupply(blockEvent.number - 1),
./ethereum-financial/src/services/aave/Aave.srv.ts:101:              `${blockEvent.number - 1}`,
./l2-bridge-zksync/src/clients/zksync_block_client.ts:72:      const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)

Explanations and references

References

  • https://figment.io/insights/ethereum-reorg-what-you-need-to-know/
  • https://www.alchemy.com/overviews/what-is-a-reorg

Helpful picture

image

TheDZhon avatar Apr 05 '24 17:04 TheDZhon

  • Eth-find: https://github.com/lidofinance/alerting-forta/pull/517

sergeyWh1te avatar Apr 08 '24 12:04 sergeyWh1te

  • [ ] ./ethereum-validators-set/src/subagents/lido-report/agent-lido-report.ts:841: blockTag: txEvent.blockNumber - 1,
  • [x] ./ethereum-validators-set/src/subagents/exitbus-oracle/agent-exitbus-oracle.ts:283: blockEvent.blockNumber - 1,
  • [x] ./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:140: blockEvent.blockNumber - 1,
  • [x] ./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:148: blockEvent.blockNumber - 1,
  • [x] ./l2-bridge-linea/src/clients/linea_block_client.ts:72: const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
  • [x] ./l2-bridge-mantle/src/clients/mantle_block_client.ts:72: const blocks = await this.mantleProvider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
  • [ ] ./voting-watcher/src/agent-voting-watcher.ts:156: const prevBlock = await ethersProvider.getBlock(blockEvent.blockNumber - 1);
  • [x] ./ethereum-steth/src/services/steth_operation/StethOperation.srv.ts:230: this.ethProvider.getBufferedEther(shiftedBlockNumber - 1),
  • [x] ./l2-bridge-base/src/services/monitor_withdrawals.ts:54: const withdrawalEvents = await this.withdrawalsClient.getWithdrawalEvents(pastl2Block, l2BlockNumber - 1)
  • [x] ./l2-bridge-base/src/clients/base_block_client.ts:72: const blocks = await this.provider.fetchL2Blocks(this.cachedBlockDto.number, latestBlock.right.number - 1)
  • [x] ./ethereum-financial/src/services/aave/Aave.srv.ts:63: this.ethProvider.getStethBalance(this.aaveAstethAddress, blockEvent.number - 1),
  • [x] ./ethereum-financial/src/services/aave/Aave.srv.ts:64: await this.ethProvider.getTotalSupply(blockEvent.number - 1),
  • [x] ./ethereum-financial/src/services/aave/Aave.srv.ts:101: ${blockEvent.number - 1},
  • [x] ./l2-bridge-zksync/src/clients/zksync_block_client.ts:72: const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)

sergeyWh1te avatar Apr 09 '24 09:04 sergeyWh1te

For ./l2-bridge-zksync/src/clients/zksync_block_client.ts:72: const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)

/l2-bridge-linea/src/clients/linea_block_client.ts:72: const blocks = await this.provider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)

./l2-bridge-mantle/src/clients/mantle_block_client.ts:72: const blocks = await this.mantleProvider.fetchBlocks(this.cachedBlockDto.number, latestBlock.right.number - 1)

For const withdrawalEvents = await this.withdrawalsClient.getWithdrawalEvents(pastl2Block, l2BlockNumber - 1) if (E.isLeft(withdrawalEvents)) { return withdrawalEvents }

./ethereum-validators-set/src/subagents/exitbus-oracle/agent-exitbus-oracle.ts:283: blockEvent.blockNumber - 1, ./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:140: blockEvent.blockNumber - 1, ./ethereum-validators-set/src/subagents/accounting-oracle/agent-accounting-oracle.ts:148: blockEvent.blockNumber - 1,

It's ok - code fetches a range of blocks.

where we really need to fix:

  • [ ] ./ethereum-validators-set/src/subagents/lido-report/agent-lido-report.ts:841: blockTag: txEvent.blockNumber - 1,
  • [ ] ./voting-watcher/src/agent-voting-watcher.ts:156: const prevBlock = await ethersProvider.getBlock(blockEvent.blockNumber - 1);

@lidofinance/lido-tooling @lidofinance/lido-dao-ops-team Pls, look at issue...

sergeyWh1te avatar Apr 09 '24 12:04 sergeyWh1te

Turns out it's not possible to use the blockHeader for the API requests at least for some services, the workaround is in #529 (tl;dr: grab the block object by the parent.blockHash and still use block.number)

More context: https://github.com/ethers-io/ethers.js/issues/2808

TheDZhon avatar Apr 15 '24 11:04 TheDZhon