Skip to content

feat: small blob fixes/improvements #11686

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.

Sign up for GitHub

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

Merged
TomAFrench merged 12 commits into from
Feb 13, 2025
Merged

feat: small blob fixes/improvements #11686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fa52b3d
fix: support multiple blobs p block
MirandaWood Jan 31, 2025
8bd6dfc
Merge remote-tracking branch 'origin/master' into mw/small-blob-fixes
MirandaWood Feb 3, 2025
28ca0bd
feat: remove some range checks, ensure archiver.test handles multiple…
MirandaWood Feb 3, 2025
679497f
chore: declare inside statements
MirandaWood Feb 4, 2025
89ab63f
feat: improve array_merge and remove a warning
MirandaWood Feb 4, 2025
c4ad2e2
feat: oops more array_merge improvement/comments
MirandaWood Feb 4, 2025
b9d0a20
Merge remote-tracking branch 'origin/master' into mw/small-blob-fixes
MirandaWood Feb 4, 2025
7dc9065
chore: add back import (the nr warning lied to me)
MirandaWood Feb 4, 2025
8b6ebfe
Merge remote-tracking branch 'origin/master' into mw/small-blob-fixes
MirandaWood Feb 7, 2025
e343018
chore: cleanup after merge
MirandaWood Feb 7, 2025
da35098
Merge remote-tracking branch 'origin/master' into mw/small-blob-fixes
MirandaWood Feb 12, 2025
55949bb
docs: add comment (ty Tom)
MirandaWood Feb 12, 2025
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
35 changes: 24 additions & 11 deletions noir-projects/noir-protocol-circuits/crates/rollup-lib/src/components.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::abis::{
use super::abis::tx_effect::TxEffect;
use dep::types::{
abis::{
log::Log, log_hash::ScopedLogHash, public_data_write::PublicDataWrite,
log_hash::ScopedLogHash, private_log::PrivateLog, public_data_write::PublicDataWrite,
public_log::PublicLog, sponge_blob::SpongeBlob,
},
constants::{
Expand Down Expand Up @@ -84,7 +84,6 @@ pub fn accumulate_blocks_fees(
// TODO(Miranda): combine fees with same recipient depending on rollup structure
// Assuming that the final rollup tree (block root -> block merge -> root) has max 32 leaves (TODO: constrain in root), then
// in the worst case, we would be checking the left 16 values (left_len = 16) against the right 16 (right_len = 16).
// Either way, construct arr in unconstrained and make use of hints to point to merged fee array.
array_merge(left.fees, right.fees)
}

Expand All @@ -101,7 +100,7 @@ pub fn accumulate_blob_public_inputs(
left_len + right_len <= AZTEC_MAX_EPOCH_DURATION,
"too many blob public input structs accumulated in rollup",
);
// NB: For some reason, the below is around 150k gates cheaper than array_merge
// NB: The below is cheaper than array_merge because assigning BlockBlobPublicInputs is cheaper than calling .equals
let mut add_from_left = true;
let mut result = [BlockBlobPublicInputs::empty(); AZTEC_MAX_EPOCH_DURATION];
for i in 0..result.len() {
Expand Down Expand Up @@ -246,12 +245,14 @@ fn get_tx_effects_hash_input(
// NOTE HASHES
array_len = array_length(note_hashes);
if array_len != 0 {
let mut check_elt = true;
let notes_prefix = encode_blob_prefix(NOTES_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], notes_prefix);
offset += 1;

for j in 0..MAX_NOTE_HASHES_PER_TX {
if j < array_len {
check_elt &= j != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[offset + j], note_hashes[j]);
}
}
Expand All @@ -261,12 +262,14 @@ fn get_tx_effects_hash_input(
// NULLIFIERS
array_len = array_length(nullifiers);
if array_len != 0 {
let mut check_elt = true;
let nullifiers_prefix = encode_blob_prefix(NULLIFIERS_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], nullifiers_prefix);
offset += 1;

for j in 0..MAX_NULLIFIERS_PER_TX {
if j < array_len {
check_elt &= j != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[offset + j], nullifiers[j]);
}
}
Expand All @@ -276,12 +279,14 @@ fn get_tx_effects_hash_input(
// L2 TO L1 MESSAGES
array_len = array_length(tx_effect.l2_to_l1_msgs);
if array_len != 0 {
let mut check_elt = true;
let l2_to_l1_msgs_prefix = encode_blob_prefix(L2_L1_MSGS_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], l2_to_l1_msgs_prefix);
offset += 1;

for j in 0..MAX_L2_TO_L1_MSGS_PER_TX {
if j < array_len {
check_elt &= j != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[offset + j], tx_effect.l2_to_l1_msgs[j]);
}
}
Expand All @@ -291,12 +296,14 @@ fn get_tx_effects_hash_input(
// PUBLIC DATA UPDATE REQUESTS
array_len = array_length(public_data_update_requests);
if array_len != 0 {
let mut check_elt = true;
let public_data_update_requests_prefix =
encode_blob_prefix(PUBLIC_DATA_UPDATE_REQUESTS_PREFIX, array_len * 2);
assert_eq(tx_effects_hash_input[offset], public_data_update_requests_prefix);
offset += 1;
for j in 0..MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX {
if j < array_len {
check_elt &= j != array_len;
if check_elt {
assert_eq(
tx_effects_hash_input[offset + j * 2],
public_data_update_requests[j].leaf_slot,
Expand All @@ -314,14 +321,16 @@ fn get_tx_effects_hash_input(
// PRIVATE_LOGS
array_len = array_length(private_logs) * PRIVATE_LOG_SIZE_IN_FIELDS;
if array_len != 0 {
let mut check_elt = true;
let private_logs_prefix = encode_blob_prefix(PRIVATE_LOGS_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], private_logs_prefix);
offset += 1;

for j in 0..MAX_PRIVATE_LOGS_PER_TX {
for k in 0..PRIVATE_LOG_SIZE_IN_FIELDS {
let index = offset + j * PRIVATE_LOG_SIZE_IN_FIELDS + k;
if index < array_len {
check_elt &= j * PRIVATE_LOG_SIZE_IN_FIELDS + k != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[index], private_logs[j].fields[k]);
}
}
Expand All @@ -332,14 +341,16 @@ fn get_tx_effects_hash_input(
// PUBLIC LOGS
array_len = array_length(public_logs) * PUBLIC_LOG_SIZE_IN_FIELDS;
if array_len != 0 {
let mut check_elt = true;
let public_logs_prefix = encode_blob_prefix(PUBLIC_LOGS_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], public_logs_prefix);
offset += 1;
for j in 0..MAX_PUBLIC_LOGS_PER_TX {
let log = public_logs[j].serialize();
for k in 0..PUBLIC_LOG_SIZE_IN_FIELDS {
let index = offset + j * PUBLIC_LOG_SIZE_IN_FIELDS + k;
if index < array_len {
check_elt &= j * PUBLIC_LOG_SIZE_IN_FIELDS + k != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[index], log[k]);
}
}
Expand All @@ -352,12 +363,14 @@ fn get_tx_effects_hash_input(
// CONTRACT CLASS LOGS
array_len = array_length(contract_class_logs);
if array_len != 0 {
let mut check_elt = true;
let contract_class_logs_prefix = encode_blob_prefix(CONTRACT_CLASS_LOGS_PREFIX, array_len);
assert_eq(tx_effects_hash_input[offset], contract_class_logs_prefix);
offset += 1;

for j in 0..MAX_CONTRACT_CLASS_LOGS_PER_TX {
if j < array_len {
check_elt &= j != array_len;
if check_elt {
assert_eq(tx_effects_hash_input[offset + j], contract_class_logs[j]);
}
}
Expand Down Expand Up @@ -390,7 +403,7 @@ unconstrained fn get_tx_effects_hash_input_helper(
nullifiers: [Field; MAX_NULLIFIERS_PER_TX],
l2_to_l1_msgs: [Field; MAX_L2_TO_L1_MSGS_PER_TX],
public_data_update_requests: [PublicDataWrite; MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX],
private_logs: [Log<PRIVATE_LOG_SIZE_IN_FIELDS>; MAX_PRIVATE_LOGS_PER_TX],
private_logs: [PrivateLog; MAX_PRIVATE_LOGS_PER_TX],
public_logs: [PublicLog; MAX_PUBLIC_LOGS_PER_TX],
contract_class_logs: [Field; MAX_CONTRACT_CLASS_LOGS_PER_TX],
revert_code: Field,
Expand Down
23 changes: 22 additions & 1 deletion noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,29 @@ pub fn array_concat<T, let N: u32, let M: u32>(array1: [T; N], array2: [T; M]) -
}
result
}

/// This function assumes that `array1` and `array2` contain no more than N non-empty elements between them,
/// if this is not the case then elements from the end of `array2` will be dropped.
pub fn array_merge<T, let N: u32>(array1: [T; N], array2: [T; N]) -> [T; N]

@TomAFrench TomAFrench Feb 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function seems a little weird in general to me as it assumes that the input arrays cannot be more than half filled, i.e. that array_length(array1) + array_length(array2) <= N.

Previously if we had array_length(array1) + array_length(array2) > N then the circuit would be unsatisfiable but this new implementation will drop any trailing elements from array2. We should add an explicit assertion on the two lengths now.

where
T: Empty + Eq,
{
/// Safety: we constrain this array below
let result = unsafe { array_merge_helper(array1, array2) };
// We assume arrays have been validated. The only use cases so far are with previously validated arrays.
let array1_len = array_length(array1);
let mut add_from_left = true;
for i in 0..N {
add_from_left &= i != array1_len;
if add_from_left {
assert_eq(result[i], array1[i]);
} else {
assert_eq(result[i], array2[i - array1_len]);
}
}
result
}

unconstrained fn array_merge_helper<T, let N: u32>(array1: [T; N], array2: [T; N]) -> [T; N]
where
T: Empty + Eq,
{
Expand Down
72 changes: 35 additions & 37 deletions yarn-project/archiver/src/archiver/archiver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Archiver', () => {
(b.header.globalVariables.timestamp = new Fr(now + DefaultL1ContractsConfig.ethereumSlotDuration * (i + 1))),
);
const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHash));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));

publicClient.getBlockNumber.mockResolvedValueOnce(2500n).mockResolvedValueOnce(2600n).mockResolvedValueOnce(2700n);

Expand All @@ -199,19 +199,19 @@ describe('Archiver', () => {

mockInbox.read.totalMessagesInserted.mockResolvedValueOnce(2n).mockResolvedValueOnce(6n);

const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

makeMessageSentEvent(98n, 1n, 0n);
makeMessageSentEvent(99n, 1n, 1n);
makeL2BlockProposedEvent(101n, 1n, blocks[0].archive.root.toString(), [blobHashes[0]]);
makeL2BlockProposedEvent(101n, 1n, blocks[0].archive.root.toString(), blobHashes[0]);

makeMessageSentEvent(2504n, 2n, 0n);
makeMessageSentEvent(2505n, 2n, 1n);
makeMessageSentEvent(2505n, 2n, 2n);
makeMessageSentEvent(2506n, 3n, 1n);
makeL2BlockProposedEvent(2510n, 2n, blocks[1].archive.root.toString(), [blobHashes[1]]);
makeL2BlockProposedEvent(2520n, 3n, blocks[2].archive.root.toString(), [blobHashes[2]]);
makeL2BlockProposedEvent(2510n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);
makeL2BlockProposedEvent(2520n, 3n, blocks[2].archive.root.toString(), blobHashes[2]);
publicClient.getTransaction.mockResolvedValueOnce(rollupTxs[0]);

rollupTxs.slice(1).forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
Expand Down Expand Up @@ -281,7 +281,7 @@ describe('Archiver', () => {
const numL2BlocksInTest = 2;

const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHash));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));

// Here we set the current L1 block number to 102. L1 to L2 messages after this should not be read.
publicClient.getBlockNumber.mockResolvedValue(102n);
Expand All @@ -295,13 +295,13 @@ describe('Archiver', () => {

makeMessageSentEvent(66n, 1n, 0n);
makeMessageSentEvent(68n, 1n, 1n);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), [blobHashes[0]]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), [blobHashes[1]]);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), blobHashes[0]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);
makeL2BlockProposedEvent(90n, 3n, badArchive, [badBlobHash]);

rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

await archiver.start(false);

Expand All @@ -326,7 +326,7 @@ describe('Archiver', () => {
const numL2BlocksInTest = 2;

const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHash));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));

publicClient.getBlockNumber.mockResolvedValueOnce(50n).mockResolvedValueOnce(100n);
mockRollup.read.status
Expand All @@ -337,12 +337,12 @@ describe('Archiver', () => {

makeMessageSentEvent(66n, 1n, 0n);
makeMessageSentEvent(68n, 1n, 1n);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), [blobHashes[0]]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), [blobHashes[1]]);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), blobHashes[0]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);

rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

await archiver.start(false);

Expand All @@ -364,7 +364,7 @@ describe('Archiver', () => {
const numL2BlocksInTest = 2;

const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHash));
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));

publicClient.getBlockNumber.mockResolvedValueOnce(50n).mockResolvedValueOnce(100n).mockResolvedValueOnce(150n);

Expand All @@ -388,12 +388,12 @@ describe('Archiver', () => {

makeMessageSentEvent(66n, 1n, 0n);
makeMessageSentEvent(68n, 1n, 1n);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), [blobHashes[0]]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), [blobHashes[1]]);
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), blobHashes[0]);
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);

rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

await archiver.start(false);

Expand Down Expand Up @@ -434,15 +434,15 @@ describe('Archiver', () => {
const l2Block = blocks[0];
l2Block.header.globalVariables.slotNumber = new Fr(notLastL2SlotInEpoch);
blocks = [l2Block];
const blobHashes = [await makeVersionedBlobHash(l2Block)];
const blobHashes = await makeVersionedBlobHashes(l2Block);

const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
publicClient.getBlockNumber.mockResolvedValueOnce(l1BlockForL2Block);
mockRollup.read.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]);
makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString(), blobHashes);
rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

await archiver.start(false);

Expand All @@ -468,16 +468,16 @@ describe('Archiver', () => {
const l2Block = blocks[0];
l2Block.header.globalVariables.slotNumber = new Fr(lastL2SlotInEpoch);
blocks = [l2Block];
const blobHashes = [await makeVersionedBlobHash(l2Block)];
const blobHashes = await makeVersionedBlobHashes(l2Block);

const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
publicClient.getBlockNumber.mockResolvedValueOnce(l1BlockForL2Block);
mockRollup.read.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]);
makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString(), blobHashes);

rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobFromBlock(b)));
blobsFromBlocks.forEach(blob => blobSinkClient.getBlobSidecar.mockResolvedValueOnce([blob]));
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));

await archiver.start(false);

Expand Down Expand Up @@ -594,22 +594,20 @@ async function makeRollupTx(l2Block: L2Block) {
}

/**
* Makes a versioned blob hash for testing purposes.
* Makes versioned blob hashes for testing purposes.
* @param l2Block - The L2 block.
* @returns A versioned blob hash.
* @returns Versioned blob hashes.
*/
async function makeVersionedBlobHash(l2Block: L2Block): Promise<`0x${string}`> {
return `0x${(await Blob.fromFields(l2Block.body.toBlobFields()))
.getEthVersionedBlobHash()
.toString('hex')}` as `0x${string}`;
async function makeVersionedBlobHashes(l2Block: L2Block): Promise<`0x${string}`[]> {
const blobHashes = (await Blob.getBlobs(l2Block.body.toBlobFields())).map(b => b.getEthVersionedBlobHash());
return blobHashes.map(h => `0x${h.toString('hex')}` as `0x${string})`);
}

/**
* Blob response to be returned from the blob sink based on the expected block.
* @param block - The block.
* @returns The blob.
* @returns The blobs.
*/
function makeBlobFromBlock(block: L2Block) {
const blob = block.body.toBlobFields();
return Blob.fromFields(blob);
async function makeBlobsFromBlock(block: L2Block) {
return await Blob.getBlobs(block.body.toBlobFields());
}
2 changes: 1 addition & 1 deletion yarn-project/archiver/src/archiver/data_retrieval.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ async function getBlockFromRollupTx(
// Body.fromBlobFields to accept blob buffers directly
let blockFields: Fr[];
try {
blockFields = blobBodies.flatMap(b => b.toEncodedFields());
blockFields = Blob.toEncodedFields(blobBodies);
} catch (err: any) {
if (err instanceof BlobDeserializationError) {
logger.fatal(err.message);
Expand Down
Loading