Skip to content
Closed
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 @@ -75,7 +75,9 @@ describe('FeePayerBalanceEvictionRule', () => {
db = mock<MerkleTreeReadOperations>();
mockWorldState.getCommitted.mockReturnValue(db);
mockWorldState.getSnapshot.mockReturnValue(db);
mockWorldState.syncImmediate.mockResolvedValue(BlockNumber(1));
mockWorldState.syncImmediate.mockImplementation((blockNumber?: number) =>
Promise.resolve(BlockNumber(blockNumber ?? 1)),
);

feePayerBalances = new Map();
setupBalances(feePayerBalances);
Expand Down Expand Up @@ -312,7 +314,7 @@ describe('FeePayerBalanceEvictionRule', () => {

await rule.evict(context, pool);

expect(mockWorldState.syncImmediate).toHaveBeenCalledWith(5);
expect(mockWorldState.syncImmediate).toHaveBeenCalledWith(5, true);
expect(mockWorldState.getSnapshot).toHaveBeenCalledWith(5);
});

Expand All @@ -337,6 +339,29 @@ describe('FeePayerBalanceEvictionRule', () => {
expect(result.success).toBe(true);
expect(result.txsEvicted).toEqual([lowMeta.txHash]);
});

it('skips eviction gracefully when block was pruned (reorg race)', async () => {
const meta = createMeta('0x1111', { feeLimit: 100n });
const txsByFeePayer = new Map([[feePayer1, [meta]]]);
const pool = createPoolOps(txsByFeePayer);
setupBalances(new Map([[feePayer1, 0n]]));

// Simulate reorg: syncImmediate returns a block number less than the target
mockWorldState.syncImmediate.mockResolvedValue(BlockNumber(3));

const context: EvictionContext = {
event: EvictionEvent.BLOCK_MINED,
block: blockHeader, // block number 5
newNullifiers: [],
feePayers: [feePayer1],
};

const result = await rule.evict(context, pool);

expect(result.success).toBe(true);
expect(result.txsEvicted).toHaveLength(0);
expect(mockWorldState.getSnapshot).not.toHaveBeenCalled();
});
});

describe('CHAIN_PRUNED events', () => {
Expand Down Expand Up @@ -368,6 +393,32 @@ describe('FeePayerBalanceEvictionRule', () => {
expect(mockWorldState.getSnapshot).toHaveBeenCalledWith(BlockNumber(3));
});

it('reports failure when snapshot data is unavailable', async () => {
const meta = createMeta('0x1111', { feeLimit: 100n });
const txsByFeePayer = new Map([[feePayer1, [meta]]]);
const pool = createPoolOps(txsByFeePayer);

// Balance insufficient - would trigger eviction if snapshot were available
setupBalances(new Map([[feePayer1, 0n]]));

// Make getSnapshot return a db that throws (simulating pruned block data)
const dbFailing = mock<MerkleTreeReadOperations>();
dbFailing.getPreviousValueIndex.mockRejectedValue(new Error('Unable to find low leaf'));
mockWorldState.getSnapshot.mockReturnValue(dbFailing);

const context: EvictionContext = {
event: EvictionEvent.CHAIN_PRUNED,
blockNumber: BlockNumber(3),
};

const result = await rule.evict(context, pool);

// Error propagates to outer catch, reported as failure
expect(result.success).toBe(false);
expect(result.txsEvicted).toHaveLength(0);
expect(result.error).toBeDefined();
});

it('evicts txs when balance changed after prune', async () => {
const meta = createMeta('0x1111', { feeLimit: 100n });
const txsByFeePayer = new Map([[feePayer1, [meta]]]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ export class FeePayerBalanceEvictionRule implements EvictionRule {

if (context.event === EvictionEvent.BLOCK_MINED) {
const blockNumber = context.block.getBlockNumber();
await this.worldState.syncImmediate(blockNumber);
const syncedTo = await this.worldState.syncImmediate(blockNumber, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this is the right approach. I suspect what happened here is

  1. Block 4425 was pruned and immediately replaced.
  2. We called await this.worldState.syncImmediate(blockNumber) and at the time, I suspect the world state was synced to the old block 4425 so happily returned.
  3. Then it processed the prune, so by the time we went to query it, the block was gone and it hadn't processed the new block.

I think if we simply call await this.worldState.syncImmediate(), with no args, this will forc the world state to do a full sync against the latest archiver state. Processing both the pruned block and the newly added replacement.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've pushed #21556 after seeing this, what do you guys think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that is the more general fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Closing this PR.

if (syncedTo < blockNumber) {
// Block was likely pruned due to a reorg. Skip eviction;
// the subsequent CHAIN_PRUNED event will re-evaluate all pending fee payers.
this.log.debug(
`Skipping BLOCK_MINED fee payer eviction: world state at block ${syncedTo}, expected ${blockNumber}`,
);
return { reason: this.reason, success: true, txsEvicted: [] };
}
return await this.evictForFeePayers(context.feePayers, this.worldState.getSnapshot(blockNumber), pool);
}

Expand All @@ -45,7 +53,7 @@ export class FeePayerBalanceEvictionRule implements EvictionRule {
txsEvicted: [],
};
} catch (err) {
this.log.error('Failed to evict txs due to fee payer balance', { err });
this.log.warn('Failed to evict txs due to fee payer balance', { err });
return {
reason: this.reason,
success: false,
Expand Down
Loading