Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: from < genesis allowed in getBlocks #2816

Merged
merged 6 commits into from
Oct 12, 2023
Merged
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
57 changes: 28 additions & 29 deletions yarn-project/archiver/src/archiver/archiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
EncodedContractFunction,
ExtendedContractData,
GetUnencryptedLogsResponse,
INITIAL_L2_BLOCK_NUM,
L1ToL2Message,
L1ToL2MessageSource,
L2Block,
Expand Down Expand Up @@ -50,17 +49,17 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
/**
* Next L1 block number to fetch `L2BlockProcessed` logs from (i.e. `fromBlock` in eth_getLogs).
*/
private nextL2BlockFromBlock = 0n;
private nextL2BlockFromL1Block = 0n;

/**
* Last Processed Block Number
*/
private lastProcessedBlockNumber = 0n;
private lastProcessedL1BlockNumber = 0n;

/**
* Use this to track logged block in order to avoid repeating the same message.
*/
private lastLoggedBlockNumber = 0n;
private lastLoggedL1BlockNumber = 0n;

/**
* Creates a new instance of the Archiver.
Expand All @@ -85,8 +84,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
private readonly pollingIntervalMs = 10_000,
private readonly log: DebugLogger = createDebugLogger('aztec:archiver'),
) {
this.nextL2BlockFromBlock = BigInt(searchStartBlock);
this.lastProcessedBlockNumber = BigInt(searchStartBlock);
this.nextL2BlockFromL1Block = BigInt(searchStartBlock);
this.lastProcessedL1BlockNumber = BigInt(searchStartBlock);
}

/**
Expand Down Expand Up @@ -140,12 +139,12 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
* @param blockUntilSynced - If true, blocks until the archiver has fully synced.
*/
private async sync(blockUntilSynced: boolean) {
const currentBlockNumber = await this.publicClient.getBlockNumber();
if (currentBlockNumber <= this.lastProcessedBlockNumber) {
const currentL1BlockNumber = await this.publicClient.getBlockNumber();
if (currentL1BlockNumber <= this.lastProcessedL1BlockNumber) {
// reducing logs, otherwise this gets triggered on every loop (1s)
if (currentBlockNumber !== this.lastLoggedBlockNumber) {
this.log(`No new blocks to process, current block number: ${currentBlockNumber}`);
this.lastLoggedBlockNumber = currentBlockNumber;
if (currentL1BlockNumber !== this.lastLoggedL1BlockNumber) {
this.log(`No new blocks to process, current block number: ${currentL1BlockNumber}`);
this.lastLoggedL1BlockNumber = currentL1BlockNumber;
}
return;
}
Expand Down Expand Up @@ -184,15 +183,15 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
this.publicClient,
this.inboxAddress,
blockUntilSynced,
this.lastProcessedBlockNumber + 1n, // + 1 to prevent re-including messages from the last processed block
currentBlockNumber,
this.lastProcessedL1BlockNumber + 1n, // + 1 to prevent re-including messages from the last processed block
currentL1BlockNumber,
);
const retrievedCancelledL1ToL2Messages = await retrieveNewCancelledL1ToL2Messages(
this.publicClient,
this.inboxAddress,
blockUntilSynced,
this.lastProcessedBlockNumber + 1n,
currentBlockNumber,
this.lastProcessedL1BlockNumber + 1n,
currentL1BlockNumber,
);

// TODO (#717): optimise this - there could be messages in confirmed that are also in pending.
Expand All @@ -204,21 +203,21 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
this.log('Removing pending l1 to l2 messages from store where messages were cancelled');
await this.store.cancelPendingL1ToL2Messages(retrievedCancelledL1ToL2Messages.retrievedData);

this.lastProcessedBlockNumber = currentBlockNumber;
this.lastProcessedL1BlockNumber = currentL1BlockNumber;

// ********** Events that are processed per block **********

// Read all data from chain and then write to our stores at the end
const nextExpectedL2BlockNum = BigInt(this.store.getBlocksLength() + INITIAL_L2_BLOCK_NUM);
const nextExpectedL2BlockNum = BigInt((await this.store.getBlockNumber()) + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this break if we restart the archiver? Say L2 is at block 30. Say archiver restarts. So nextExpectedL2BlockNum would be 1. But I suppose we don't care about re-syncing until testnet?

Copy link
Member

Choose a reason for hiding this comment

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

It would have been one in the first instance too?

Copy link
Contributor Author

@benesjan benesjan Oct 12, 2023

Choose a reason for hiding this comment

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

This seems correct to me because when we start archiver the next expected L2 block num is 1 because that's the first block.

this.log(
`Retrieving chain state from L1 block: ${this.nextL2BlockFromBlock}, next expected l2 block number: ${nextExpectedL2BlockNum}`,
`Retrieving chain state from L1 block: ${this.nextL2BlockFromL1Block}, next expected l2 block number: ${nextExpectedL2BlockNum}`,
);
const retrievedBlocks = await retrieveBlocks(
this.publicClient,
this.rollupAddress,
blockUntilSynced,
this.nextL2BlockFromBlock,
currentBlockNumber,
this.nextL2BlockFromL1Block,
currentL1BlockNumber,
nextExpectedL2BlockNum,
);

Expand All @@ -231,8 +230,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
this.publicClient,
this.contractDeploymentEmitterAddress,
blockUntilSynced,
this.nextL2BlockFromBlock,
currentBlockNumber,
this.nextL2BlockFromL1Block,
currentL1BlockNumber,
blockHashMapping,
);
if (retrievedBlocks.retrievedData.length === 0) {
Expand Down Expand Up @@ -270,14 +269,14 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource

// store retrieved L2 blocks after removing new logs information.
// remove logs to serve "lightweight" block information. Logs can be fetched separately if needed.
await this.store.addL2Blocks(
await this.store.addBlocks(
retrievedBlocks.retrievedData.map(block =>
L2Block.fromFields(omit(block, ['newEncryptedLogs', 'newUnencryptedLogs']), block.getBlockHash()),
),
);

// set the L1 block for the next search
this.nextL2BlockFromBlock = retrievedBlocks.nextEthBlockNumber;
this.nextL2BlockFromL1Block = retrievedBlocks.nextEthBlockNumber;
}

/**
Expand Down Expand Up @@ -306,21 +305,21 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource
* @param limit - The number of blocks to return.
* @returns The requested L2 blocks.
*/
public getL2Blocks(from: number, limit: number): Promise<L2Block[]> {
return this.store.getL2Blocks(from, limit);
public getBlocks(from: number, limit: number): Promise<L2Block[]> {
return this.store.getBlocks(from, limit);
}

/**
* Gets an l2 block.
* @param number - The block number to return (inclusive).
* @returns The requested L2 block.
*/
public async getL2Block(number: number): Promise<L2Block | undefined> {
public async getBlock(number: number): Promise<L2Block | undefined> {
// If the number provided is -ve, then return the latest block.
if (number < 0) {
number = this.store.getBlocksLength();
number = await this.store.getBlockNumber();
}
const blocks = await this.store.getL2Blocks(number, 1);
const blocks = await this.store.getBlocks(number, 1);
return blocks.length === 0 ? undefined : blocks[0];
}

Expand Down
30 changes: 19 additions & 11 deletions yarn-project/archiver/src/archiver/archiver_store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('Archiver Memory Store', () => {
const blocks = Array(10)
.fill(0)
.map((_, index) => L2Block.random(index));
await archiverStore.addL2Blocks(blocks);
await archiverStore.addBlocks(blocks);
// Offset indices by INITIAL_L2_BLOCK_NUM to ensure we are correctly aligned
for (const [from, limit] of [
[0 + INITIAL_L2_BLOCK_NUM, 10],
Expand All @@ -35,7 +35,7 @@ describe('Archiver Memory Store', () => {
[11 + INITIAL_L2_BLOCK_NUM, 1],
]) {
const expected = blocks.slice(from - INITIAL_L2_BLOCK_NUM, from - INITIAL_L2_BLOCK_NUM + limit);
const actual = await archiverStore.getL2Blocks(from, limit);
const actual = await archiverStore.getBlocks(from, limit);
expect(expected).toEqual(actual);
}
});
Expand Down Expand Up @@ -64,10 +64,20 @@ describe('Archiver Memory Store', () => {
const blocks = Array(10)
.fill(0)
.map((_, index) => L2Block.random(index));
await archiverStore.addL2Blocks(blocks);
await expect(async () => await archiverStore.getL2Blocks(1, 0)).rejects.toThrow(
`Invalid block range from: 1, limit: 0`,
);
await archiverStore.addBlocks(blocks);
await expect(async () => await archiverStore.getBlocks(1, 0)).rejects.toThrow(`Invalid limit: 0`);
});

it('returns from the beginning when "from" < genesis block', async () => {
const blocks = Array(10)
.fill(0)
.map((_, index) => L2Block.random(index));
await archiverStore.addBlocks(blocks);
const from = -5;
const limit = 1;
const retrievedBlocks = await archiverStore.getBlocks(from, limit);
expect(retrievedBlocks.length).toEqual(1);
expect(retrievedBlocks[0]).toEqual(blocks[0]);
});

test.each([LogType.ENCRYPTED, LogType.UNENCRYPTED])(
Expand All @@ -77,9 +87,7 @@ describe('Archiver Memory Store', () => {
.fill(0)
.map(_ => L2BlockL2Logs.random(6, 3, 2));
await archiverStore.addLogs(logs, logType);
await expect(async () => await archiverStore.getLogs(1, 0, logType)).rejects.toThrow(
`Invalid block range from: 1, limit: 0`,
);
await expect(async () => await archiverStore.getLogs(1, 0, logType)).rejects.toThrow(`Invalid limit: 0`);
},
);

Expand All @@ -91,7 +99,7 @@ describe('Archiver Memory Store', () => {
.fill(0)
.map((_, index: number) => L2Block.random(index + 1, 4, 2, 3, 2, 2));

await archiverStore.addL2Blocks(blocks);
await archiverStore.addBlocks(blocks);
await archiverStore.addLogs(
blocks.map(block => block.newUnencryptedLogs!),
LogType.UNENCRYPTED,
Expand All @@ -118,7 +126,7 @@ describe('Archiver Memory Store', () => {
L2Block.random(index + 1, txsPerBlock, 2, numPublicFunctionCalls, 2, numUnencryptedLogs),
);

await archiverStore.addL2Blocks(blocks);
await archiverStore.addBlocks(blocks);
await archiverStore.addLogs(
blocks.map(block => block.newUnencryptedLogs!),
LogType.UNENCRYPTED,
Expand Down
37 changes: 13 additions & 24 deletions yarn-project/archiver/src/archiver/archiver_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ export interface ArchiverDataStore {
* @param blocks - The L2 blocks to be added to the store.
* @returns True if the operation is successful.
*/
addL2Blocks(blocks: L2Block[]): Promise<boolean>;
addBlocks(blocks: L2Block[]): Promise<boolean>;

/**
* Gets up to `limit` amount of L2 blocks starting from `from`.
* @param from - Number of the first block to return (inclusive).
* @param limit - The number of blocks to return.
* @returns The requested L2 blocks.
*/
getL2Blocks(from: number, limit: number): Promise<L2Block[]>;
getBlocks(from: number, limit: number): Promise<L2Block[]>;

/**
* Gets an l2 tx.
Expand Down Expand Up @@ -150,12 +150,6 @@ export interface ArchiverDataStore {
* @returns The number of the latest L2 block processed.
*/
getBlockNumber(): Promise<number>;

/**
* Gets the length of L2 blocks in store.
* @returns The length of L2 Blocks stored.
*/
getBlocksLength(): number;
}

/**
Expand Down Expand Up @@ -215,7 +209,7 @@ export class MemoryArchiverStore implements ArchiverDataStore {
* @param blocks - The L2 blocks to be added to the store.
* @returns True if the operation is successful (always in this implementation).
*/
public addL2Blocks(blocks: L2Block[]): Promise<boolean> {
public addBlocks(blocks: L2Block[]): Promise<boolean> {
this.l2BlockContexts.push(...blocks.map(block => new L2BlockContext(block)));
this.l2Txs.push(...blocks.flatMap(b => b.getTxs()));
return Promise.resolve(true);
Expand Down Expand Up @@ -299,18 +293,21 @@ export class MemoryArchiverStore implements ArchiverDataStore {
* @param from - Number of the first block to return (inclusive).
* @param limit - The number of blocks to return.
* @returns The requested L2 blocks.
* @remarks When "from" is smaller than genesis block number, blocks from the beginning are returned.
*/
public getL2Blocks(from: number, limit: number): Promise<L2Block[]> {
public getBlocks(from: number, limit: number): Promise<L2Block[]> {
// Return an empty array if we are outside of range
if (limit < 1) {
throw new Error(`Invalid block range from: ${from}, limit: ${limit}`);
throw new Error(`Invalid limit: ${limit}`);
}
if (from < INITIAL_L2_BLOCK_NUM || from > this.l2BlockContexts.length) {

const fromIndex = Math.max(from - INITIAL_L2_BLOCK_NUM, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

we use INITIAL_L2_BLOCK_NUM here but not in the archiver? Is that an issue?

Copy link
Member

Choose a reason for hiding this comment

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

It was being used in an overloaded context in the archiver. ( incrementing to next block / getting the first block number )

if (fromIndex >= this.l2BlockContexts.length) {
return Promise.resolve([]);
}
const startIndex = from - INITIAL_L2_BLOCK_NUM;
const endIndex = startIndex + limit;
return Promise.resolve(this.l2BlockContexts.slice(startIndex, endIndex).map(blockContext => blockContext.block));

const toIndex = fromIndex + limit;
return Promise.resolve(this.l2BlockContexts.slice(fromIndex, toIndex).map(blockContext => blockContext.block));
}

/**
Expand Down Expand Up @@ -354,7 +351,7 @@ export class MemoryArchiverStore implements ArchiverDataStore {
*/
getLogs(from: number, limit: number, logType: LogType): Promise<L2BlockL2Logs[]> {
if (from < INITIAL_L2_BLOCK_NUM || limit < 1) {
throw new Error(`Invalid block range from: ${from}, limit: ${limit}`);
throw new Error(`Invalid limit: ${limit}`);
}
const logs = logType === LogType.ENCRYPTED ? this.encryptedLogsPerBlock : this.unencryptedLogsPerBlock;
if (from > logs.length) {
Expand Down Expand Up @@ -513,12 +510,4 @@ export class MemoryArchiverStore implements ArchiverDataStore {
if (this.l2BlockContexts.length === 0) return Promise.resolve(INITIAL_L2_BLOCK_NUM - 1);
return Promise.resolve(this.l2BlockContexts[this.l2BlockContexts.length - 1].block.number);
}

/**
* Gets the length of L2 blocks in store.
* @returns The length of L2 Blocks array.
*/
public getBlocksLength(): number {
return this.l2BlockContexts.length;
}
}
6 changes: 3 additions & 3 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class AztecNodeService implements AztecNode {
* @returns The blocks requested.
*/
public async getBlock(number: number): Promise<L2Block | undefined> {
return await this.blockSource.getL2Block(number);
return await this.blockSource.getBlock(number);
}

/**
Expand All @@ -169,7 +169,7 @@ export class AztecNodeService implements AztecNode {
* @returns The blocks requested.
*/
public async getBlocks(from: number, limit: number): Promise<L2Block[]> {
return (await this.blockSource.getL2Blocks(from, limit)) ?? [];
return (await this.blockSource.getBlocks(from, limit)) ?? [];
}

/**
Expand Down Expand Up @@ -401,7 +401,7 @@ export class AztecNodeService implements AztecNode {
this.log.info(`Simulating tx ${await tx.getTxHash()}`);
const blockNumber = (await this.blockSource.getBlockNumber()) + 1;
const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber));
const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty();
const prevGlobalVariables = (await this.blockSource.getBlock(-1))?.globalVariables ?? GlobalVariables.empty();

// Instantiate merkle trees so uncommitted updates by this simulation are local to it.
// TODO we should be able to remove this after https://github.com/AztecProtocol/aztec-packages/issues/1869
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/p2p/src/client/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class MockBlockSource implements L2BlockSource {
* @param number - The block number to return (inclusive).
* @returns The requested L2 block.
*/
public getL2Block(number: number) {
public getBlock(number: number) {
return Promise.resolve(this.l2Blocks[number]);
}

Expand All @@ -55,7 +55,7 @@ export class MockBlockSource implements L2BlockSource {
* @param limit - The maximum number of blocks to return.
* @returns The requested mocked L2 blocks.
*/
public getL2Blocks(from: number, limit: number) {
public getBlocks(from: number, limit: number) {
return Promise.resolve(this.l2Blocks.slice(from, from + limit));
}

Expand Down
2 changes: 1 addition & 1 deletion yarn-project/p2p/src/client/p2p_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export class P2PClient implements P2P {
// start looking for further blocks
const blockProcess = async () => {
while (!this.stopping) {
const blocks = await this.blockDownloader.getL2Blocks();
const blocks = await this.blockDownloader.getBlocks();
await this.handleL2Blocks(blocks);
}
};
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class Sequencer {
this.state = SequencerState.CREATING_BLOCK;

const newGlobalVariables = await this.globalsBuilder.buildGlobalVariables(new Fr(blockNumber));
const prevGlobalVariables = (await this.l2BlockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty();
const prevGlobalVariables = (await this.l2BlockSource.getBlock(-1))?.globalVariables ?? GlobalVariables.empty();

// Process txs and drop the ones that fail processing
// We create a fresh processor each time to reset any cached state (eg storage writes)
Expand Down
Loading