Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import { ProcessNewTransferResultStatusEnum } from '../enums/ProcessNewTransferR
import Contract from 'web3-eth-contract';
import { SubmissionEntity } from '../../../../entities/SubmissionEntity';

const CHAINS_WITH_FINALIZED_BLOCK = new Set([
56, // BSC
137, // Polygon
43114, // Avalanche
]);

@Injectable()
export class AddNewEventsAction {
private readonly logger = new Logger(AddNewEventsAction.name);
Expand Down Expand Up @@ -71,7 +77,7 @@ export class AddNewEventsAction {
web3.eth.setProvider = gateContract.setProvider;

// Set block range: use provided values or defaults
const toBlock = to ?? (await this.getConfirmedBlockNumber(web3, chainConfig.blockConfirmation));
const toBlock = to ?? (await this.getConfirmedBlockNumber(web3, chainConfig));
let fromBlock = from ?? supportedChain.latestBlock;
Comment on lines +80 to 81

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

When transitioning a chain to use finalized blocks, or if the finalized block temporarily lags behind the head, the finalized block number (toBlock) can be less than the last scanned block (fromBlock).

Under the current implementation, if fromBlock > toBlock, the stale RPC protection logic on line 96 will be triggered, which rolls back the latestBlock in the database to safeBlock (the last event block or toBlock). This causes the node to perform redundant scanning of already processed blocks once the finalized block catches up.

Since finalized blocks are guaranteed not to reorg, we do not need to roll back the progress. Instead, we should cap toBlock to be at least fromBlock for finalized chains, which safely pauses scanning without triggering a database rollback.

Suggested change
const toBlock = to ?? (await this.getConfirmedBlockNumber(web3, chainConfig));
let fromBlock = from ?? supportedChain.latestBlock;
let toBlock = to ?? (await this.getConfirmedBlockNumber(web3, chainConfig));
let fromBlock = from ?? supportedChain.latestBlock;
if (CHAINS_WITH_FINALIZED_BLOCK.has(chainConfig.chainId) && toBlock < fromBlock) {
toBlock = fromBlock;
}

logger.debug(`Getting events from block ${fromBlock} to ${toBlock} on ${supportedChain.network}`);

Expand Down Expand Up @@ -150,15 +156,26 @@ export class AddNewEventsAction {
/**
* Retrieves the block number with the specified number of confirmations.
* @param web3 - An instance of Web3Custom to interact with the blockchain.
* @param blockConfirmation - The number of blocks to subtract from the latest block to ensure confirmation.
* @param chainConfig - EVM chain config with confirmation and chain identity details.
* @returns A promise resolving to the block number that has the specified number of confirmations.
*/
private async getConfirmedBlockNumber(web3: Web3Custom, blockConfirmation: number): Promise<number> {
private async getConfirmedBlockNumber(web3: Web3Custom, chainConfig: EvmChainConfig): Promise<number> {
// Get the latest block number from the RPC (the most recent block in the blockchain)
const lastRPCBlock = await web3.eth.getBlockNumber();
this.logger.debug(`Last rpc block is ${lastRPCBlock}`);
// Subtract the confirmation count to get the block number considered confirmed
return lastRPCBlock - blockConfirmation;
// Subtract the confirmation count to get the block number considered confirmed.
const confirmationsBlock = lastRPCBlock - chainConfig.blockConfirmation;

if (!CHAINS_WITH_FINALIZED_BLOCK.has(chainConfig.chainId)) {
return confirmationsBlock;
}

const finalizedBlock = await web3.eth.getBlock('finalized');
this.logger.debug(
`Using finalized block for chainId ${chainConfig.chainId}: finalized=${finalizedBlock.number}; confirmed=${confirmationsBlock}`,
);

return finalizedBlock.number;
}

async getEvents(gateContract: Contract, fromBlock: number, toBlock: number) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ import { getRepositoryToken } from '@nestjs/typeorm';
import { Test, TestingModule } from '@nestjs/testing';
import { SupportedChainEntity } from '../../../../../entities/SupportedChainEntity';

jest.mock('@debridge-finance/solana-utils', () => ({
constants: {
SOLANA_CHAIN_ID: 7565164,
},
crypto: {
hashSubmissionIdRaw: jest.fn().mockReturnValue('submission-id'),
},
}));

/**
* Creates an AddNewEventsAction service with configurable mocks.
*
Expand All @@ -19,6 +28,8 @@ import { SupportedChainEntity } from '../../../../../entities/SupportedChainEnti
* @param lastEventBlockNumber - highest blockNumber in submissions table, null if no events
* @param events - raw events returned by getPastEvents (default: [])
* @param blockConfirmation - number of blocks subtracted from RPC head (default: 1)
* @param finalizedBlockNumber - optional finalized block number returned by RPC
* @param finalizedBlockError - optional error thrown by finalized block RPC call
*/
async function buildService(config: {
chainId: number;
Expand All @@ -28,6 +39,8 @@ async function buildService(config: {
lastEventBlockNumber?: number | null;
events?: any[];
blockConfirmation?: number;
finalizedBlockNumber?: number;
finalizedBlockError?: Error;
}) {
const isSolana = config.isSolana ?? false;
const blockConfirmation = config.blockConfirmation ?? 1;
Expand All @@ -38,6 +51,9 @@ async function buildService(config: {
const getPastEventsMock = jest.fn().mockResolvedValue(events);
const processMock = jest.fn().mockResolvedValue({});
const syncTransactionsMock = jest.fn().mockResolvedValue({});
const getFinalizedBlockMock = config.finalizedBlockError
? jest.fn().mockRejectedValue(config.finalizedBlockError)
: jest.fn().mockResolvedValue({ number: config.finalizedBlockNumber });
const submissionsFindOneMock = jest.fn().mockResolvedValue(
lastEventBlockNumber !== null
? { blockNumber: lastEventBlockNumber, chainFrom: config.chainId }
Expand All @@ -52,6 +68,7 @@ async function buildService(config: {
getPastEvents: getPastEventsMock,
})),
getBlockNumber: jest.fn().mockResolvedValue(config.rpcBlockNumber),
getBlock: getFinalizedBlockMock,
},
};

Expand Down Expand Up @@ -114,6 +131,7 @@ async function buildService(config: {
web3: web3Mock,
updateMock,
getPastEventsMock,
getFinalizedBlockMock,
processMock,
syncTransactionsMock,
};
Expand Down Expand Up @@ -211,6 +229,69 @@ describe('AddNewEventsAction', () => {
);
expect(processMock).toBeCalledTimes(1);
});

it('Polygon scans up to finalized block when it is ahead of confirmations fallback', async () => {
const { service, chainId, getPastEventsMock, getFinalizedBlockMock } = await buildService({
chainId: 137,
latestBlock: 1000,
rpcBlockNumber: 1300,
blockConfirmation: 256,
finalizedBlockNumber: 1200,
});

await service.action(chainId);

expect(getFinalizedBlockMock).toBeCalledWith('finalized');
expect(getPastEventsMock).toBeCalledWith('Sent', {
fromBlock: 1000,
toBlock: 1200,
});
});

it('Polygon uses finalized block even when it is behind confirmations fallback', async () => {
const { service, chainId, getPastEventsMock } = await buildService({
chainId: 137,
latestBlock: 1000,
rpcBlockNumber: 1300,
blockConfirmation: 256,
finalizedBlockNumber: 1020,
});

await service.action(chainId);

expect(getPastEventsMock).toBeCalledWith('Sent', {
fromBlock: 1000,
toBlock: 1020,
});
});
Comment on lines +251 to +266

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure that the scanner does not trigger a database rollback or perform redundant scans when the finalized block is behind the latest scanned block, we should add a dedicated unit test verifying this behavior.

    it('Polygon uses finalized block even when it is behind confirmations fallback', async () => {
      const { service, chainId, getPastEventsMock } = await buildService({
        chainId: 137,
        latestBlock: 1000,
        rpcBlockNumber: 1300,
        blockConfirmation: 256,
        finalizedBlockNumber: 1020,
      });

      await service.action(chainId);

      expect(getPastEventsMock).toBeCalledWith('Sent', {
        fromBlock: 1000,
        toBlock: 1020,
      });
    });

    it('Polygon does not roll back or scan when finalized block is behind latest scanned block', async () => {
      const { service, chainId, getPastEventsMock, updateMock } = await buildService({
        chainId: 137,
        latestBlock: 1000,
        rpcBlockNumber: 1300,
        blockConfirmation: 256,
        finalizedBlockNumber: 980,
      });

      await service.action(chainId);

      expect(getPastEventsMock).not.toBeCalled();
      expect(updateMock).not.toBeCalled();
    });


it('Polygon stops scanning when finalized block RPC fails', async () => {
const { service, chainId, getPastEventsMock, processMock, updateMock } = await buildService({
chainId: 137,
latestBlock: 1000,
rpcBlockNumber: 1300,
blockConfirmation: 256,
finalizedBlockError: new Error('finalized is not supported'),
});

await service.action(chainId);

expect(getPastEventsMock).not.toBeCalled();
expect(processMock).not.toBeCalled();
expect(updateMock).not.toBeCalled();
});

it('does not request finalized block for regular EVM chains', async () => {
const { service, chainId, getFinalizedBlockMock } = await buildService({
chainId: 1,
latestBlock: 98,
rpcBlockNumber: 100,
});

await service.action(chainId);

expect(getFinalizedBlockMock).not.toBeCalled();
});
});

/**
Expand All @@ -231,7 +312,7 @@ describe('AddNewEventsAction', () => {
// safeBlock = Math.max(50, 99) = 99 — picks toBlock, minimal rollback.
it('RPC slightly behind, last event below toBlock — uses toBlock', async () => {
const { service, chainId, updateMock, getPastEventsMock, processMock } = await buildService({
chainId: 56,
chainId: 1,
latestBlock: 200,
rpcBlockNumber: 100,
lastEventBlockNumber: 50,
Expand Down Expand Up @@ -281,7 +362,7 @@ describe('AddNewEventsAction', () => {
// safeBlock = Math.max(0, 99) = 99 — toBlock is the only reference point.
it('no events in DB — falls back to toBlock', async () => {
const { service, chainId, updateMock } = await buildService({
chainId: 137,
chainId: 42161,
latestBlock: 200,
rpcBlockNumber: 100,
lastEventBlockNumber: null,
Expand Down