diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp index cf55d7c5f6ec..150540a270ca 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state.cpp @@ -781,7 +781,9 @@ bool WorldStateWrapper::sync_block(msgpack::object& obj, msgpack::sbuffer& buf) request.value.paddedNoteHashes, request.value.paddedL1ToL2Messages, request.value.paddedNullifiers, - request.value.publicDataWrites); + request.value.publicDataWrites, + request.value.expectedArchiveRoot, + request.value.expectedPreviousArchiveRoot); MsgHeader header(request.header.messageId); messaging::TypedMessage resp_msg(WorldStateMessageType::SYNC_BLOCK, header, { status }); diff --git a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp index 8f6b481ad41a..176e7192ce91 100644 --- a/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp +++ b/barretenberg/cpp/src/barretenberg/nodejs_module/world_state/world_state_message.hpp @@ -248,6 +248,8 @@ struct SyncBlockRequest { block_number_t blockNumber; StateReference blockStateRef; bb::fr blockHeaderHash; + bb::fr expectedArchiveRoot; + bb::fr expectedPreviousArchiveRoot; std::vector paddedNoteHashes, paddedL1ToL2Messages; std::vector paddedNullifiers; std::vector publicDataWrites; @@ -255,6 +257,8 @@ struct SyncBlockRequest { SERIALIZATION_FIELDS(blockNumber, blockStateRef, blockHeaderHash, + expectedArchiveRoot, + expectedPreviousArchiveRoot, paddedNoteHashes, paddedL1ToL2Messages, paddedNullifiers, diff --git a/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp b/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp index 883fefbd0e9b..e2076c3144c5 100644 --- a/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp +++ b/barretenberg/cpp/src/barretenberg/world_state/world_state.cpp @@ -602,11 +602,33 @@ WorldStateStatusFull WorldState::sync_block(const StateReference& block_state_re const std::vector& notes, const std::vector& l1_to_l2_messages, const std::vector& nullifiers, - const std::vector& public_writes) + const std::vector& public_writes, + const std::optional& expected_archive_root, + const std::optional& expected_previous_archive_root) { validate_trees_are_equally_synched(); rollback(); + // The archive tree is an append-only accumulator of block header hashes, so a single bad leaf (e.g. from a + // mishandled reorg) is never self-corrected: every later root stays noncanonical while the other state trees + // can re-converge from block effects. The checks further down only verify the appended leaf is the tip and + // that the four non-archive trees match the block state reference — neither catches a divergent archive root. + // So verify the local archive root against canonical both before appending (the parent root must equal the + // block's lastArchive) and after (the resulting root must equal the block's archive), failing before commit + // so the divergence is never persisted. + if (expected_previous_archive_root.has_value()) { + const bb::fr actual_previous_archive_root = + get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + if (actual_previous_archive_root != expected_previous_archive_root.value()) { + throw std::runtime_error( + format("Can't sync block: local archive root ", + actual_previous_archive_root, + " does not match the block's previous archive root ", + expected_previous_archive_root.value(), + "; world state has diverged from the canonical chain and must be resynced")); + } + } + Fork::SharedPtr fork = retrieve_fork(CANONICAL_FORK_ID); Signal signal(static_cast(fork->_trees.size())); std::atomic_bool success = true; @@ -681,6 +703,21 @@ WorldStateStatusFull WorldState::sync_block(const StateReference& block_state_re throw std::runtime_error("Can't synch block: block state does not match world state"); } + // The archive tree is not part of the block state reference (see is_same_state_reference), so verify the + // resulting archive root against the canonical block's archive root explicitly. + if (expected_archive_root.has_value()) { + const bb::fr actual_archive_root = + get_tree_info(WorldStateRevision::uncommitted(), MerkleTreeId::ARCHIVE).meta.root; + if (actual_archive_root != expected_archive_root.value()) { + throw std::runtime_error( + format("Can't sync block: resulting archive root ", + actual_archive_root, + " does not match the block's archive root ", + expected_archive_root.value(), + "; world state has diverged from the canonical chain and must be resynced")); + } + } + std::pair result = commit(status); if (!result.first) { throw std::runtime_error(result.second); diff --git a/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp b/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp index 2e493e2efbdf..c145138bc4e2 100644 --- a/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp +++ b/barretenberg/cpp/src/barretenberg/world_state/world_state.hpp @@ -300,7 +300,9 @@ class WorldState { const std::vector& notes, const std::vector& l1_to_l2_messages, const std::vector& nullifiers, - const std::vector& public_writes); + const std::vector& public_writes, + const std::optional& expected_archive_root = std::nullopt, + const std::optional& expected_previous_archive_root = std::nullopt); uint32_t checkpoint(const uint64_t& forkId); void commit_checkpoint(const uint64_t& forkId); diff --git a/barretenberg/cpp/src/barretenberg/world_state/world_state.test.cpp b/barretenberg/cpp/src/barretenberg/world_state/world_state.test.cpp index 1d24634e9de0..4b6181fc9c98 100644 --- a/barretenberg/cpp/src/barretenberg/world_state/world_state.test.cpp +++ b/barretenberg/cpp/src/barretenberg/world_state/world_state.test.cpp @@ -640,6 +640,74 @@ TEST_F(WorldStateTest, SyncExternalBlockFromEmpty) std::runtime_error); } +TEST_F(WorldStateTest, SyncBlockRejectsDivergentArchiveRoot) +{ + StateReference block_state_ref = { + { MerkleTreeId::NULLIFIER_TREE, + { fr("0x2e2e2d8b72294a440c728a646f01476624063f0b50dcfe293cc0fc26bef9e311"), 129 } }, + { MerkleTreeId::NOTE_HASH_TREE, + { fr("0x25c4ef02ba2bec9490376d5b56b8f1a8e5bcf5ecff91636e76660b68c2a9952d"), 1 } }, + { MerkleTreeId::PUBLIC_DATA_TREE, + { fr("0x1e2d8d1c3ea2449b3e4787d8295df3f137e08b56e891c006b3d93faef56ca3df"), 129 } }, + { MerkleTreeId::L1_TO_L2_MESSAGE_TREE, + { fr("0x22c6f7877092ecea5b313b22515e31f2e1e37349b787da10eff298800e3c7c0c"), 1 } }, + }; + + // Learn the canonical previous (genesis) and resulting archive roots from an untracked sync. + bb::fr previous_root; + bb::fr resulting_root; + { + WorldState scratch( + thread_pool_size, data_dir, map_size, tree_heights, tree_prefill, initial_header_generator_point); + previous_root = scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + scratch.sync_block( + block_state_ref, fr(1), { 42 }, { 43 }, { NullifierLeafValue(144) }, { { PublicDataLeafValue(145, 1) } }); + resulting_root = scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + } + + std::string data_dir2 = random_temp_directory(); + std::filesystem::create_directories(data_dir2); + WorldState ws(thread_pool_size, data_dir2, map_size, tree_heights, tree_prefill, initial_header_generator_point); + + // A wrong previous archive root is rejected before any leaves are appended. + EXPECT_THROW(ws.sync_block(block_state_ref, + fr(1), + { 42 }, + { 43 }, + { NullifierLeafValue(144) }, + { { PublicDataLeafValue(145, 1) } }, + resulting_root, + previous_root + fr(1)), + std::runtime_error); + + // A wrong resulting archive root is rejected before commit. + EXPECT_THROW(ws.sync_block(block_state_ref, + fr(1), + { 42 }, + { 43 }, + { NullifierLeafValue(144) }, + { { PublicDataLeafValue(145, 1) } }, + resulting_root + fr(1), + previous_root), + std::runtime_error); + + // Both rejections rolled back cleanly: world state is still at the genesis archive root. + EXPECT_EQ(ws.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root, previous_root); + + // Matching roots are accepted and advance the chain. + WorldStateStatusFull status = ws.sync_block(block_state_ref, + fr(1), + { 42 }, + { 43 }, + { NullifierLeafValue(144) }, + { { PublicDataLeafValue(145, 1) } }, + resulting_root, + previous_root); + WorldStateStatusSummary expected(1, 0, 1, true); + EXPECT_EQ(status.summary, expected); + EXPECT_EQ(ws.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root, resulting_root); +} + TEST_F(WorldStateTest, SyncBlockFromDirtyState) { WorldState ws(thread_pool_size, data_dir, map_size, tree_heights, tree_prefill, initial_header_generator_point); @@ -947,3 +1015,71 @@ TEST_F(WorldStateTest, GetBlockForIndex) EXPECT_EQ(blockNumbers[0].value(), 1); } } + +// Demonstrates the bug: syncing an empty block with a bogus block_header_hash succeeds when the optional +// expected-archive-root arguments are omitted. The 4-tree state-ref check passes (nothing changed), but +// the wrong hash is committed to the ARCHIVE, silently diverging from the canonical chain. +TEST_F(WorldStateTest, SyncEmptyBlockAcceptsBogusHashWithoutArchiveCheck) +{ + // Learn the canonical previous and resulting archive roots from an empty-block sync on a scratch instance. + bb::fr previous_archive_root; + bb::fr canonical_archive_root; + { + WorldState scratch( + thread_pool_size, data_dir, map_size, tree_heights, tree_prefill, initial_header_generator_point); + previous_archive_root = scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + StateReference genesis_state_ref = scratch.get_state_reference(WorldStateRevision::committed()); + scratch.sync_block(genesis_state_ref, fr(1), {}, {}, {}, {}); + canonical_archive_root = + scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + } + + std::string data_dir2 = random_temp_directory(); + std::filesystem::create_directories(data_dir2); + WorldState ws(thread_pool_size, data_dir2, map_size, tree_heights, tree_prefill, initial_header_generator_point); + + StateReference genesis_state_ref = ws.get_state_reference(WorldStateRevision::committed()); + + // Sync an empty block using a bogus hash — no expected-archive-root arguments provided. + EXPECT_NO_THROW(ws.sync_block(genesis_state_ref, fr(42), {}, {}, {}, {})); + + bb::fr actual_archive_root = ws.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + + // The bogus hash committed successfully, so the resulting archive root differs from the canonical one. + EXPECT_NE(actual_archive_root, canonical_archive_root); + EXPECT_NE(actual_archive_root, previous_archive_root); +} + +// Demonstrates the fix: supplying the canonical expected archive roots causes sync_block to reject the bogus +// block_header_hash for an empty block, and rolls back cleanly. +TEST_F(WorldStateTest, SyncEmptyBlockRejectsBogusHashWhenArchiveRootsAreChecked) +{ + // Learn the canonical previous and resulting archive roots from an empty-block sync on a scratch instance. + bb::fr previous_archive_root; + bb::fr canonical_archive_root; + { + WorldState scratch( + thread_pool_size, data_dir, map_size, tree_heights, tree_prefill, initial_header_generator_point); + previous_archive_root = scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + StateReference genesis_state_ref = scratch.get_state_reference(WorldStateRevision::committed()); + scratch.sync_block(genesis_state_ref, fr(1), {}, {}, {}, {}); + canonical_archive_root = + scratch.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root; + } + + std::string data_dir2 = random_temp_directory(); + std::filesystem::create_directories(data_dir2); + WorldState ws(thread_pool_size, data_dir2, map_size, tree_heights, tree_prefill, initial_header_generator_point); + + StateReference genesis_state_ref = ws.get_state_reference(WorldStateRevision::committed()); + + // Sync an empty block using a bogus hash, but supply the canonical archive roots for validation. + // The resulting archive root will not match canonical_archive_root, so this must throw. + EXPECT_THROW( + ws.sync_block(genesis_state_ref, fr(42), {}, {}, {}, {}, canonical_archive_root, previous_archive_root), + std::runtime_error); + + // The committed archive root must be unchanged (rollback was clean). + EXPECT_EQ(ws.get_tree_info(WorldStateRevision::committed(), MerkleTreeId::ARCHIVE).meta.root, + previous_archive_root); +} diff --git a/barretenberg/cpp/src/barretenberg/wsdb/wsdb_commands.hpp b/barretenberg/cpp/src/barretenberg/wsdb/wsdb_commands.hpp index 5ccf8cc760bc..3a9610bf1c3d 100644 --- a/barretenberg/cpp/src/barretenberg/wsdb/wsdb_commands.hpp +++ b/barretenberg/cpp/src/barretenberg/wsdb/wsdb_commands.hpp @@ -320,6 +320,8 @@ struct WsdbSyncBlock { block_number_t blockNumber; StateReference blockStateRef; bb::fr blockHeaderHash; + bb::fr expectedArchiveRoot; + bb::fr expectedPreviousArchiveRoot; std::vector paddedNoteHashes; std::vector paddedL1ToL2Messages; std::vector paddedNullifiers; @@ -328,6 +330,8 @@ struct WsdbSyncBlock { SERIALIZATION_FIELDS(blockNumber, blockStateRef, blockHeaderHash, + expectedArchiveRoot, + expectedPreviousArchiveRoot, paddedNoteHashes, paddedL1ToL2Messages, paddedNullifiers, diff --git a/barretenberg/cpp/src/barretenberg/wsdb/wsdb_execute.cpp b/barretenberg/cpp/src/barretenberg/wsdb/wsdb_execute.cpp index 5a6282b9de8f..41d231157929 100644 --- a/barretenberg/cpp/src/barretenberg/wsdb/wsdb_execute.cpp +++ b/barretenberg/cpp/src/barretenberg/wsdb/wsdb_execute.cpp @@ -303,8 +303,14 @@ WsdbRollback::Response WsdbRollback::execute(WsdbRequest& request) && WsdbSyncBlock::Response WsdbSyncBlock::execute(WsdbRequest& request) && { - WorldStateStatusFull status = request.world_state.sync_block( - blockStateRef, blockHeaderHash, paddedNoteHashes, paddedL1ToL2Messages, paddedNullifiers, publicDataWrites); + WorldStateStatusFull status = request.world_state.sync_block(blockStateRef, + blockHeaderHash, + paddedNoteHashes, + paddedL1ToL2Messages, + paddedNullifiers, + publicDataWrites, + expectedArchiveRoot, + expectedPreviousArchiveRoot); return Response{ .status = status }; } diff --git a/spartan/aztec-node/templates/_pod-template.yaml b/spartan/aztec-node/templates/_pod-template.yaml index 6fe273d616ff..d79611a3062d 100644 --- a/spartan/aztec-node/templates/_pod-template.yaml +++ b/spartan/aztec-node/templates/_pod-template.yaml @@ -221,6 +221,8 @@ spec: value: "{{ .Values.node.proverRealProofs }}" - name: SENTINEL_ENABLED value: "{{ .Values.node.sentinel.enabled }}" + - name: OFFENSE_COLLECTION_ENABLED + value: "{{ .Values.node.offenseCollection.enabled }}" {{- if .Values.node.slash.validatorsAlways }} - name: SLASH_VALIDATORS_ALWAYS value: {{ join "," .Values.node.slash.validatorsAlways | quote }} diff --git a/spartan/aztec-node/values.yaml b/spartan/aztec-node/values.yaml index 13102e678139..1edc93612e8a 100644 --- a/spartan/aztec-node/values.yaml +++ b/spartan/aztec-node/values.yaml @@ -147,6 +147,8 @@ node: sentinel: enabled: true + offenseCollection: + enabled: true slash: # Validator allowlists/denylists validatorsAlways: [] diff --git a/spartan/environments/block-capacity.env b/spartan/environments/block-capacity.env index af194cabfd25..6dcad9ac9045 100644 --- a/spartan/environments/block-capacity.env +++ b/spartan/environments/block-capacity.env @@ -18,6 +18,7 @@ REDEPLOY_ROLLUP_CONTRACTS=true ETHEREUM_CHAIN_ID=1337 LABS_INFRA_MNEMONIC="test test test test test test test test test test test junk" FUNDING_PRIVATE_KEY="0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" +SPONSORED_FPC=true OTEL_COLLECTOR_ENDPOINT=REPLACE_WITH_GCP_SECRET diff --git a/spartan/environments/network-defaults.yml b/spartan/environments/network-defaults.yml index 54986aaa2632..4240d118fbd9 100644 --- a/spartan/environments/network-defaults.yml +++ b/spartan/environments/network-defaults.yml @@ -206,6 +206,8 @@ _prodlike: &prodlike #--------------------------------------------------------------------------- # Enable sentinel monitoring. SENTINEL_ENABLED: true + # Enable offense collection (watchers + read-only slasher) on non-validator nodes. + OFFENSE_COLLECTION_ENABLED: true # Network presets selected via NETWORK env var; individual values can still be overridden. networks: diff --git a/spartan/scripts/deploy_network.sh b/spartan/scripts/deploy_network.sh index 1046cd2d31b5..67dd45bc19da 100755 --- a/spartan/scripts/deploy_network.sh +++ b/spartan/scripts/deploy_network.sh @@ -592,6 +592,7 @@ PROVER_MNEMONIC = "${LABS_INFRA_MNEMONIC}" PROVER_PUBLISHER_MNEMONIC_START_INDEX = ${PROVER_PUBLISHER_MNEMONIC_START_INDEX} PROVER_PUBLISHERS_PER_PROVER = ${PUBLISHERS_PER_PROVER} SENTINEL_ENABLED = ${SENTINEL_ENABLED:-null} +OFFENSE_COLLECTION_ENABLED = ${OFFENSE_COLLECTION_ENABLED:-null} SLASH_INACTIVITY_TARGET_PERCENTAGE = ${SLASH_INACTIVITY_TARGET_PERCENTAGE:-null} SLASH_INACTIVITY_PENALTY = ${SLASH_INACTIVITY_PENALTY:-null} SLASH_DATA_WITHHOLDING_PENALTY = ${SLASH_DATA_WITHHOLDING_PENALTY:-null} diff --git a/spartan/terraform/deploy-aztec-infra/main.tf b/spartan/terraform/deploy-aztec-infra/main.tf index 97f76e9d7cc8..e5c1e5fc5b8b 100644 --- a/spartan/terraform/deploy-aztec-infra/main.tf +++ b/spartan/terraform/deploy-aztec-infra/main.tf @@ -191,6 +191,7 @@ locals { "validator.publisherMnemonicStartIndex" = var.VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX "validator.node.env.COINBASE" = var.VALIDATOR_COINBASE "validator.sentinel.enabled" = var.SENTINEL_ENABLED + "validator.offenseCollection.enabled" = var.OFFENSE_COLLECTION_ENABLED "validator.slash.inactivityTargetPercentage" = var.SLASH_INACTIVITY_TARGET_PERCENTAGE "validator.slash.inactivityPenalty" = var.SLASH_INACTIVITY_PENALTY "validator.slash.dataWithholdingPenalty" = var.SLASH_DATA_WITHHOLDING_PENALTY diff --git a/spartan/terraform/deploy-aztec-infra/variables.tf b/spartan/terraform/deploy-aztec-infra/variables.tf index af2e98b46555..b47886cdb80a 100644 --- a/spartan/terraform/deploy-aztec-infra/variables.tf +++ b/spartan/terraform/deploy-aztec-infra/variables.tf @@ -436,6 +436,12 @@ variable "SENTINEL_ENABLED" { default = true } +variable "OFFENSE_COLLECTION_ENABLED" { + description = "Whether to enable offense collection (watchers + read-only slasher) on non-validator nodes" + type = string + default = true +} + variable "SLASH_INACTIVITY_TARGET_PERCENTAGE" { description = "The slash inactivity target percentage" type = string diff --git a/yarn-project/archiver/src/modules/data_store_updater.test.ts b/yarn-project/archiver/src/modules/data_store_updater.test.ts index 73dbf6cae211..4a2c8f659e3d 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.test.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.test.ts @@ -1,11 +1,15 @@ +import { CONTRACT_CLASS_LOG_SIZE_IN_FIELDS, CONTRACT_CLASS_PUBLISHED_MAGIC_VALUE } from '@aztec/constants'; import { BlockNumber, CheckpointNumber, IndexWithinCheckpoint, SlotNumber } from '@aztec/foundation/branded-types'; import { Fr } from '@aztec/foundation/curves/bn254'; import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; +import { ProtocolContractAddress } from '@aztec/protocol-contracts'; import { ContractClassPublishedEvent } from '@aztec/protocol-contracts/class-registry'; import { ContractInstancePublishedEvent } from '@aztec/protocol-contracts/instance-registry'; +import { BundledProtocolContractsProvider } from '@aztec/protocol-contracts/providers/bundle'; +import { bufferAsFields } from '@aztec/stdlib/abi'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { GENESIS_BLOCK_HEADER_HASH, L2Block } from '@aztec/stdlib/block'; -import { ContractClassLog, PrivateLog } from '@aztec/stdlib/logs'; +import { ContractClassLog, ContractClassLogFields, PrivateLog } from '@aztec/stdlib/logs'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; import '@aztec/stdlib/testing/jest'; import { BlockHeader } from '@aztec/stdlib/tx'; @@ -15,11 +19,40 @@ import { readFileSync } from 'fs'; import { dirname, resolve } from 'path'; import { fileURLToPath } from 'url'; +import { registerProtocolContracts } from '../factory.js'; import { type ArchiverDataStores, createArchiverDataStores } from '../store/data_stores.js'; import { L2TipsCache } from '../store/l2_tips_cache.js'; import { makeCheckpoint, makePublishedCheckpoint } from '../test/mock_structs.js'; import { ArchiverDataStoreUpdater } from './data_store_updater.js'; +/** + * Builds a ContractClassPublished log for a real bundled protocol contract class. The log carries the + * protocol contract's actual fields so that the class id the data store updater recomputes matches the + * bundled protocol class id (otherwise the updater would skip it as a mismatched id). + */ +function buildProtocolContractClassLog(contractClass: { + artifactHash: Fr; + privateFunctionsRoot: Fr; + packedBytecode: Buffer; + id: Fr; +}): ContractClassLog { + const fields = [ + new Fr(CONTRACT_CLASS_PUBLISHED_MAGIC_VALUE), + contractClass.id, + new Fr(1), // version + contractClass.artifactHash, + contractClass.privateFunctionsRoot, + // The remaining fields encode the packed bytecode; size it to fill the rest of the log so that + // ContractClassPublishedEvent.fromLog reads back the full bytecode buffer. + ...bufferAsFields(contractClass.packedBytecode, CONTRACT_CLASS_LOG_SIZE_IN_FIELDS - 5), + ]; + return new ContractClassLog( + ProtocolContractAddress.ContractClassRegistry, + new ContractClassLogFields(fields), + fields.length, + ); +} + /** Loads the sample ContractClassPublished event payload from protocol-contracts fixtures. */ function getSampleContractClassPublishedEventPayload(): Buffer { const fixturePath = resolve( @@ -84,6 +117,38 @@ describe('ArchiverDataStoreUpdater', () => { expect(retrievedInstance?.address.equals(instanceAddress)).toBe(true); }); + it('treats an on-chain re-publish of a preloaded protocol contract class as idempotent (A-1257)', async () => { + // Protocol contracts are preloaded at synthetic block 0 via registerProtocolContracts. When a + // bundled protocol contract class is later (re-)published on chain, the archiver must not throw + // when re-adding the already-present class, which would otherwise stall L1 sync. + await registerProtocolContracts(store); + + const provider = new BundledProtocolContractsProvider(); + const protocolContract = await provider.getProtocolContractArtifact('ContractClassRegistry'); + const protocolClassId = protocolContract.contractClass.id; + + // The class is queryable from the block-0 preload. + expect(await store.contractClasses.getContractClass(protocolClassId)).toBeDefined(); + + // Build a block whose tx emits a ContractClassPublished log for the bundled protocol class id. + const block = await L2Block.random(BlockNumber(1), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + }); + block.body.txEffects[0].contractClassLogs = [buildProtocolContractClassLog(protocolContract.contractClass)]; + + // Sanity check: the log decodes to the expected protocol class id (so the updater does not skip it). + expect( + ContractClassPublishedEvent.fromLog(block.body.txEffects[0].contractClassLogs[0]).contractClassId.equals( + protocolClassId, + ), + ).toBe(true); + + // Adding the block must not throw, and the protocol class must remain queryable afterwards. + await expect(updater.addProposedBlock(block)).resolves.not.toThrow(); + expect(await store.contractClasses.getContractClass(protocolClassId)).toBeDefined(); + }); + it('removes contract class and instance data when blocks are pruned via setCheckpointData', async () => { // First, add a local provisional block with contract data const localBlock = await L2Block.random(BlockNumber(1), { diff --git a/yarn-project/archiver/src/store/contract_class_store.test.ts b/yarn-project/archiver/src/store/contract_class_store.test.ts index 42a389c84031..375bd3bb7320 100644 --- a/yarn-project/archiver/src/store/contract_class_store.test.ts +++ b/yarn-project/archiver/src/store/contract_class_store.test.ts @@ -1,6 +1,7 @@ import { BlockNumber } from '@aztec/foundation/branded-types'; import { Fr } from '@aztec/foundation/curves/bn254'; import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; +import { ProtocolContractClassId } from '@aztec/protocol-contracts'; import { type ContractClassPublic, type ContractClassPublicWithCommitment, @@ -58,4 +59,41 @@ describe('ContractClassStore', () => { await expect(contractClassStore.getContractClass(Fr.random())).resolves.toBeUndefined(); }); }); + + describe('protocol contract classes (A-1257)', () => { + // Protocol contracts are preloaded at synthetic block 0. A later on-chain (re-)publish of a + // bundled protocol class id must be treated as a no-op rather than a hard error, and must never + // delete the preloaded entry. + let protocolClass: ContractClassPublic; + const preloadBlock = 0; + + beforeEach(async () => { + const base = await makeContractClassPublic(); + protocolClass = { ...base, id: ProtocolContractClassId.ContractClassRegistry }; + await contractClassStore.addContractClasses([await withCommitment(protocolClass)], BlockNumber(preloadBlock)); + }); + + it('treats re-publish of a preloaded protocol class as a no-op and keeps it queryable', async () => { + const originalCommitment = await computePublicBytecodeCommitment(protocolClass.packedBytecode); + await expect( + contractClassStore.addContractClasses([await withCommitment(protocolClass)], BlockNumber(50)), + ).resolves.not.toThrow(); + await expect(contractClassStore.getContractClass(protocolClass.id)).resolves.toMatchObject(protocolClass); + // The block-0 preload must be left untouched: the re-publish must not clobber the stored bytecode commitment. + await expect(contractClassStore.getBytecodeCommitment(protocolClass.id)).resolves.toEqual(originalCommitment); + }); + + it('does not delete a protocol class', async () => { + await contractClassStore.deleteContractClasses([protocolClass], BlockNumber(preloadBlock)); + await expect(contractClassStore.getContractClass(protocolClass.id)).resolves.toMatchObject(protocolClass); + }); + + it('still throws when a non-protocol class is added twice', async () => { + const nonProtocolClass = await makeContractClassPublic(123); + await contractClassStore.addContractClasses([await withCommitment(nonProtocolClass)], BlockNumber(10)); + await expect( + contractClassStore.addContractClasses([await withCommitment(nonProtocolClass)], BlockNumber(11)), + ).rejects.toThrow(/already exists/); + }); + }); }); diff --git a/yarn-project/archiver/src/store/contract_class_store.ts b/yarn-project/archiver/src/store/contract_class_store.ts index ea3b5a48ef56..09e27c73c5fd 100644 --- a/yarn-project/archiver/src/store/contract_class_store.ts +++ b/yarn-project/archiver/src/store/contract_class_store.ts @@ -2,6 +2,7 @@ import { Fr } from '@aztec/foundation/curves/bn254'; import { toArray } from '@aztec/foundation/iterable'; import { BufferReader, numToUInt8, serializeToBuffer } from '@aztec/foundation/serialize'; import type { AztecAsyncKVStore, AztecAsyncMap } from '@aztec/kv-store'; +import { isProtocolContractClass } from '@aztec/protocol-contracts'; import type { ContractClassPublic, ContractClassPublicWithBlockNumber, @@ -50,6 +51,12 @@ export class ContractClassStore { await this.db.transactionAsync(async () => { const key = contractClass.id.toString(); if (await this.#contractClasses.hasAsync(key)) { + // Protocol contracts are preloaded at block 0, so a later on-chain (re-)publish of a bundled + // protocol class id is valid and must be a no-op. Keep the existing block-0 entry untouched + // (do not bump its block number) so it survives reorgs of the publishing block. + if (isProtocolContractClass(contractClass.id)) { + return; + } throw new Error(`Contract class ${key} already exists, cannot add again at block ${blockNumber}`); } await this.#contractClasses.set( @@ -61,6 +68,11 @@ export class ContractClassStore { } async deleteContractClass(contractClass: ContractClassPublic, blockNumber: number): Promise { + // Protocol contracts are preloaded at block 0 and must never be deleted, even when the block that + // (re-)published them on-chain is unwound by a reorg. + if (isProtocolContractClass(contractClass.id)) { + return; + } const restoredContractClass = await this.#contractClasses.getAsync(contractClass.id.toString()); if (restoredContractClass && deserializeContractClassPublic(restoredContractClass).l2BlockNumber >= blockNumber) { await this.db.transactionAsync(async () => { diff --git a/yarn-project/archiver/src/store/contract_instance_store.test.ts b/yarn-project/archiver/src/store/contract_instance_store.test.ts index f6330c2ed0a2..2cd7490945c6 100644 --- a/yarn-project/archiver/src/store/contract_instance_store.test.ts +++ b/yarn-project/archiver/src/store/contract_instance_store.test.ts @@ -1,6 +1,7 @@ import { BlockNumber } from '@aztec/foundation/branded-types'; import { Fr } from '@aztec/foundation/curves/bn254'; import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; +import { ProtocolContractAddress } from '@aztec/protocol-contracts'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { type ContractInstanceWithAddress, SerializableContractInstance } from '@aztec/stdlib/contract'; import '@aztec/stdlib/testing/jest'; @@ -55,6 +56,58 @@ describe('ContractInstanceStore', () => { }); }); + describe('protocol contract instances (A-1257)', () => { + // Protocol contracts are preloaded at synthetic block 0. A later on-chain (re-)publish of a + // bundled protocol instance must be treated as a no-op rather than a hard error, and must never + // delete the preloaded entry. + let protocolInstance: ContractInstanceWithAddress; + const timestamp = 3600n; + const preloadBlock = 0; + + beforeEach(async () => { + const classId = Fr.random(); + const randomInstance = await SerializableContractInstance.random({ + currentContractClassId: classId, + originalContractClassId: classId, + }); + protocolInstance = { ...randomInstance, address: ProtocolContractAddress.ContractClassRegistry }; + await contractInstanceStore.addContractInstances([protocolInstance], BlockNumber(preloadBlock)); + }); + + it('treats re-publish of a preloaded protocol instance as a no-op and keeps it queryable', async () => { + await expect( + contractInstanceStore.addContractInstances([protocolInstance], BlockNumber(50)), + ).resolves.not.toThrow(); + await expect( + contractInstanceStore.getContractInstance(protocolInstance.address, timestamp), + ).resolves.toMatchObject(protocolInstance); + // The block-0 preload must be left untouched: the re-publish must not bump the recorded deployment block. + await expect( + contractInstanceStore.getContractInstanceDeploymentBlockNumber(protocolInstance.address), + ).resolves.toEqual(preloadBlock); + }); + + it('does not delete a protocol instance', async () => { + await contractInstanceStore.deleteContractInstances([protocolInstance]); + await expect( + contractInstanceStore.getContractInstance(protocolInstance.address, timestamp), + ).resolves.toMatchObject(protocolInstance); + }); + + it('still throws when a non-protocol instance is added twice', async () => { + const classId = Fr.random(); + const randomInstance = await SerializableContractInstance.random({ + currentContractClassId: classId, + originalContractClassId: classId, + }); + const nonProtocolInstance = { ...randomInstance, address: await AztecAddress.random() }; + await contractInstanceStore.addContractInstances([nonProtocolInstance], BlockNumber(10)); + await expect(contractInstanceStore.addContractInstances([nonProtocolInstance], BlockNumber(11))).rejects.toThrow( + /already exists/, + ); + }); + }); + describe('contractInstanceUpdates', () => { let contractInstance: ContractInstanceWithAddress; let classId: Fr; diff --git a/yarn-project/archiver/src/store/contract_instance_store.ts b/yarn-project/archiver/src/store/contract_instance_store.ts index eed2aa55b681..3c7da25f9b9d 100644 --- a/yarn-project/archiver/src/store/contract_instance_store.ts +++ b/yarn-project/archiver/src/store/contract_instance_store.ts @@ -1,5 +1,6 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; import type { AztecAsyncKVStore, AztecAsyncMap } from '@aztec/kv-store'; +import { isProtocolContract } from '@aztec/protocol-contracts'; import type { AztecAddress } from '@aztec/stdlib/aztec-address'; import { type ContractInstanceUpdateWithAddress, @@ -72,6 +73,11 @@ export class ContractInstanceStore { return this.db.transactionAsync(async () => { const key = contractInstance.address.toString(); if (await this.#contractInstances.hasAsync(key)) { + // Protocol contracts are preloaded at block 0, so a later on-chain (re-)publish of a bundled + // protocol instance is valid and must be a no-op. Keep the existing block-0 entry untouched. + if (isProtocolContract(contractInstance.address)) { + return; + } throw new Error( `Contract instance at ${key} already exists (deployed at block ${await this.#contractInstancePublishedAt.getAsync(key)}), cannot add again at block ${blockNumber}`, ); @@ -82,6 +88,11 @@ export class ContractInstanceStore { } deleteContractInstance(contractInstance: ContractInstanceWithAddress): Promise { + // Protocol contracts are preloaded at block 0 and must never be deleted, even when the block that + // (re-)published them on-chain is unwound by a reorg. + if (isProtocolContract(contractInstance.address)) { + return Promise.resolve(); + } return this.db.transactionAsync(async () => { await this.#contractInstances.delete(contractInstance.address.toString()); await this.#contractInstancePublishedAt.delete(contractInstance.address.toString()); diff --git a/yarn-project/aztec-node/src/aztec-node/config.ts b/yarn-project/aztec-node/src/aztec-node/config.ts index 6e97bb6d9773..2c4bd6d545b7 100644 --- a/yarn-project/aztec-node/src/aztec-node/config.ts +++ b/yarn-project/aztec-node/src/aztec-node/config.ts @@ -60,6 +60,8 @@ export type AztecNodeConfig = ArchiverConfig & debugForceTxProofVerification: boolean; /** Whether to enable the prover node as a subsystem. */ enableProverNode: boolean; + /** Whether to run the slashing watchers to collect offenses even if not a validator. */ + enableOffenseCollection: boolean; /** * Test-only: use the deterministic AutomineSequencer instead of the production Sequencer. * Requires `aztecTargetCommitteeSize === 0` on the deployed rollup and anvil-backed L1. @@ -111,6 +113,11 @@ export const aztecNodeConfigMappings: ConfigMappingsType = { description: 'Whether to enable the prover node as a subsystem.', ...booleanConfigHelper(false), }, + enableOffenseCollection: { + env: 'OFFENSE_COLLECTION_ENABLED', + description: 'Whether to run the slashing watchers to collect offenses even if not a validator.', + ...booleanConfigHelper(false), + }, useAutomineSequencer: { env: 'USE_AUTOMINE_SEQUENCER', description: 'Test-only: use AutomineSequencer instead of the production Sequencer.', diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 045b2ed80dbb..b66f9683dd81 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -153,6 +153,7 @@ import { FullNodeCheckpointsBuilder as CheckpointsBuilder, FullNodeCheckpointsBuilder, NodeKeystoreAdapter, + ProposalHandler, ValidatorClient, createProposalHandler, createValidatorClient, @@ -683,10 +684,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb const globalVariableBuilder = new GlobalVariableBuilder(publicClient, globalVariableBuilderConfig); const feeProvider = new FeeProviderImpl(dateProvider, publicClient, globalVariableBuilderConfig); - const proverOnly = config.enableProverNode && config.disableValidator; - if (proverOnly) { - log.info('Starting in prover-only mode: skipping validator, sequencer, sentinel, and slasher subsystems'); - } + const collectOffenses = !config.disableValidator || config.enableOffenseCollection; // create the tx pool and the p2p client, which will need the l2 block source const p2pClient = await createP2PClient( @@ -726,6 +724,10 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb let validatorClient: ValidatorClient | undefined; + // The proposal handler (validator-owned or standalone) tracks invalid-proposal/equivocation slots and + // feeds the attested-invalid-proposal watcher, so the watcher works on non-validator nodes too. + let proposalHandler: ProposalHandler | undefined; + // Tracks successful checkpoint re-execution by a checkpoint proposal handler. const reexecutionTracker = new CheckpointReexecutionTracker(); @@ -754,7 +756,8 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb const vc = validatorClient; const getValidatorAddresses = () => vc.getValidatorAddresses().map(a => a.toString()); - validatorClient.getProposalHandler().register(p2pClient, true, archiver, getValidatorAddresses); + proposalHandler = validatorClient.getProposalHandler(); + proposalHandler.register(p2pClient, true, archiver, getValidatorAddresses); if (!options.dontStartSequencer) { await validatorClient.registerHandlers(); @@ -769,7 +772,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb if (!validatorClient) { const reexecute = !!config.alwaysReexecuteBlockProposals; log.info(`Setting up proposal handler` + (reexecute ? ' with reexecution of proposals' : '')); - createProposalHandler(config, { + proposalHandler = createProposalHandler(config, { checkpointsBuilder: validatorCheckpointsBuilder, worldState: worldStateSynchronizer, epochCache, @@ -780,7 +783,8 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb dateProvider, telemetry, reexecutionTracker, - }).register(p2pClient, reexecute, archiver); + }); + proposalHandler.register(p2pClient, reexecute, archiver); } // Start world state and wait for it to sync to the archiver. @@ -789,19 +793,18 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb // Start p2p. Note that it depends on world state to be running. await p2pClient.start(); - let validatorsSentinel: Awaited> | undefined; let dataWithholdingWatcher: DataWithholdingWatcher | undefined; let attestationsBlockWatcher: AttestationsBlockWatcher | undefined; let attestedInvalidProposalWatcher: AttestedInvalidProposalWatcher | undefined; let broadcastedInvalidCheckpointProposalWatcher: BroadcastedInvalidCheckpointProposalWatcher | undefined; let checkpointEquivocationWatcher: CheckpointEquivocationWatcher | undefined; - if (!proverOnly) { - validatorsSentinel = await createSentinel(epochCache, archiver, p2pClient, reexecutionTracker, config); - if (validatorsSentinel) { - watchers.push(validatorsSentinel); - } + const validatorsSentinel = await createSentinel(epochCache, archiver, p2pClient, reexecutionTracker, config); + if (validatorsSentinel) { + watchers.push(validatorsSentinel); + } + if (collectOffenses) { dataWithholdingWatcher = new DataWithholdingWatcher( epochCache, archiver, @@ -821,10 +824,12 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb ); watchers.push(broadcastedInvalidCheckpointProposalWatcher); - if (validatorClient) { + // The proposal handler (validator-owned or standalone) is the source of invalid-proposal/equivocation + // slots, so the watcher runs on non-validator offense collectors too. + if (proposalHandler) { attestedInvalidProposalWatcher = new AttestedInvalidProposalWatcher( p2pClient, - validatorClient, + proposalHandler, archiver, epochCache, config, @@ -872,9 +877,10 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb let sequencer: SequencerClient | undefined; let automineSequencer: AutomineSequencer | undefined; let slasherClient: SlasherClientInterface | undefined; - if (!config.disableValidator && validatorClient) { - // We create a slasher only if we have a sequencer, since all slashing actions go through the sequencer publisher - // as they are executed when the node is selected as proposer. + + // The slasher can run standalone to collect offenses for non-validators; it only writes to L1 when a + // proposer is elected (which requires a sequencer), so running it read-only on a non-validator is safe. + if (collectOffenses) { const validatorAddresses = keyStoreManager ? NodeKeystoreAdapter.fromKeyStoreManager(keyStoreManager).getAddresses() : []; @@ -891,7 +897,9 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb ); await slasherClient.start(); started.push(slasherClient); + } + if (!config.disableValidator && validatorClient) { const l1TxUtils = config.sequencerPublisherForwarderAddress ? await createForwarderL1TxUtilsFromSigners( publicClient, diff --git a/yarn-project/aztec-node/src/sentinel/factory.ts b/yarn-project/aztec-node/src/sentinel/factory.ts index 244d54a66a45..59cf47afdcf2 100644 --- a/yarn-project/aztec-node/src/sentinel/factory.ts +++ b/yarn-project/aztec-node/src/sentinel/factory.ts @@ -5,7 +5,7 @@ import type { P2PClient } from '@aztec/p2p'; import type { L2BlockSource } from '@aztec/stdlib/block'; import type { CheckpointReexecutionTracker } from '@aztec/stdlib/checkpoint'; import type { ChainConfig } from '@aztec/stdlib/config'; -import type { SlasherConfig } from '@aztec/stdlib/interfaces/server'; +import type { SlasherConfig, ValidatorClientConfig } from '@aztec/stdlib/interfaces/server'; import type { DataStoreConfig } from '@aztec/stdlib/kv-store'; import type { SentinelConfig } from './config.js'; @@ -17,12 +17,24 @@ export async function createSentinel( archiver: L2BlockSource, p2p: P2PClient, reexecutionTracker: CheckpointReexecutionTracker, - config: SentinelConfig & DataStoreConfig & SlasherConfig & Pick, + config: SentinelConfig & + DataStoreConfig & + SlasherConfig & + Pick & + Pick, logger = createLogger('node:sentinel'), ): Promise { - if (!config.sentinelEnabled) { + const runsValidator = !config.disableValidator; + if (!runsValidator && !config.sentinelEnabled) { + logger.verbose('Sentinel is disabled'); return undefined; } + if (runsValidator) { + logger.info('Enabling sentinel since this node runs a validator'); + } else { + logger.info('Enabling sentinel from SENTINEL_ENABLED configuration'); + } + const kvStore = await createStore('sentinel', SentinelStore.SCHEMA_VERSION, config, logger.getBindings()); const storeHistoryLength = config.sentinelHistoryLengthInEpochs * epochCache.getL1Constants().epochDuration; const storeHistoricEpochPerformanceLength = config.sentinelHistoricEpochPerformanceLengthInEpochs; diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index 9d2fa2a35412..9c77a97ebffd 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -216,6 +216,7 @@ export type EnvVar = | 'RPC_MAX_BODY_SIZE' | 'RPC_SIMULATE_PUBLIC_MAX_GAS_LIMIT' | 'RPC_SIMULATE_PUBLIC_MAX_DEBUG_LOG_MEMORY_READS' + | 'OFFENSE_COLLECTION_ENABLED' | 'SENTINEL_ENABLED' | 'SENTINEL_HISTORY_LENGTH_IN_EPOCHS' | 'SENTINEL_HISTORIC_EPOCH_PERFORMANCE_LENGTH_IN_EPOCHS' diff --git a/yarn-project/protocol-contracts/src/protocol_contract.ts b/yarn-project/protocol-contracts/src/protocol_contract.ts index 6544cdab1209..75c7d43fe86b 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract.ts @@ -1,8 +1,9 @@ +import type { Fr } from '@aztec/foundation/curves/bn254'; import type { ContractArtifact } from '@aztec/stdlib/abi'; import type { AztecAddress } from '@aztec/stdlib/aztec-address'; import type { ContractClassIdPreimage, ContractClassWithId, ContractInstanceWithAddress } from '@aztec/stdlib/contract'; -import { ProtocolContractAddress } from './protocol_contract_data.js'; +import { ProtocolContractAddress, ProtocolContractClassId } from './protocol_contract_data.js'; /** Represents a canonical contract in the protocol. */ export interface ProtocolContract { @@ -20,4 +21,11 @@ export function isProtocolContract(address: AztecAddress) { return Object.values(ProtocolContractAddress).some(a => a.equals(address)); } +const protocolContractClassIds = new Set(Object.values(ProtocolContractClassId).map(id => id.toString())); + +/** Returns whether the given contract class id belongs to a bundled protocol contract. */ +export function isProtocolContractClass(classId: Fr): boolean { + return protocolContractClassIds.has(classId.toString()); +} + export { type ProtocolContractsProvider } from './provider/protocol_contracts_provider.js'; diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 317a72adbc93..08941b93de57 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -13,6 +13,7 @@ import { import { pick } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/curves/bn254'; import { TimeoutError } from '@aztec/foundation/error'; +import { FifoSet } from '@aztec/foundation/fifo-set'; import type { LogData } from '@aztec/foundation/log'; import { createLogger } from '@aztec/foundation/log'; import { retryUntil } from '@aztec/foundation/retry'; @@ -152,7 +153,49 @@ type BlockProposalSlotValidationResult = | { isValid: true } | { isValid: false; reason: 'block_proposal_beyond_checkpoint' | 'checkpoint_proposal_equivocation' }; -/** Handles block and checkpoint proposals for both validator and non-validator nodes. */ +const MAX_TRACKED_INVALID_PROPOSAL_SLOTS = 1000; + +/** Block-proposal validation failures that constitute a slashable invalid-block offense. */ +export const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ + 'state_mismatch', + 'failed_txs', + 'global_variables_mismatch', + 'invalid_proposal', + 'parent_block_wrong_slot', + 'in_hash_mismatch', +]; + +/** Checkpoint-proposal validation failures that constitute a slashable invalid-checkpoint offense. */ +export const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record< + CheckpointProposalValidationFailureReason, + boolean +> = { + // enabled + ['invalid_fee_asset_price_modifier']: true, + ['checkpoint_header_mismatch']: true, + // These late mismatches should normally be caught by earlier checks, but if reached after validating the local + // checkpoint inputs, the proposer-signed payload disagrees with deterministic recomputation. + ['archive_mismatch']: true, + ['out_hash_mismatch']: true, + ['no_blocks_for_slot']: true, + ['too_many_blocks_in_checkpoint']: true, + ['checkpoint_validation_failed']: true, + ['last_block_archive_mismatch']: true, + + // disabled + ['invalid_signature']: false, + ['last_block_not_found']: false, + ['block_fetch_error']: false, + ['checkpoint_already_published']: false, +}; + +/** + * Handles block and checkpoint proposals for both validator and non-validator nodes. Also tracks which slots + * had a slashable invalid proposal or a proposal equivocation, exposing them via the + * `InvalidProposalSlotSource` interface consumed by the attested-invalid-proposal slashing watcher. The + * tracking is populated as a side effect of validating/re-executing proposals, so any node that re-executes + * proposals (the default) can serve it — not only validators. + */ export class ProposalHandler { public readonly tracer: Tracer; @@ -175,6 +218,12 @@ export class ProposalHandler { private checkpointProposalValidationFailureCallback?: CheckpointProposalValidationFailureCallback; + /** Slots at which a slashable invalid block or checkpoint proposal was observed. */ + private readonly slotsWithInvalidProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); + + /** Slots at which a proposal equivocation was observed; suppresses attested-to-invalid-proposal slashing. */ + private readonly slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); + constructor( private checkpointsBuilder: FullNodeCheckpointsBuilder, private worldState: WorldStateSynchronizer, @@ -221,6 +270,26 @@ export class ProposalHandler { this.reexecutionTracker.recordOutcome(slot, archive, 'valid', checkpointNumber); } + /** Whether a slashable invalid block or checkpoint proposal was observed at the given slot (InvalidProposalSlotSource). */ + public hasInvalidProposals(slotNumber: SlotNumber): boolean { + return this.slotsWithInvalidProposals.has(slotNumber); + } + + /** Whether a proposal equivocation was observed at the given slot (InvalidProposalSlotSource). */ + public hasProposalEquivocation(slotNumber: SlotNumber): boolean { + return this.slotsWithProposalEquivocation.has(slotNumber); + } + + /** Records a slot as having a slashable invalid proposal, for offense observers (sentinel/slasher watchers). */ + public markInvalidProposalSlot(slotNumber: SlotNumber): void { + this.slotsWithInvalidProposals.add(slotNumber); + } + + /** Records a slot as having a proposal equivocation, which suppresses attested-to-invalid-proposal slashing. */ + public markProposalEquivocation(slotNumber: SlotNumber): void { + this.slotsWithProposalEquivocation.add(slotNumber); + } + /** * Registers handlers for block and checkpoint proposals on the p2p client. * Records the p2p client so validation can inspect retained proposals. @@ -256,6 +325,18 @@ export class ProposalHandler { }); return true; } else { + // Track invalid proposals / equivocations so offense observers (the attested-invalid-proposal + // watcher) work on non-validator nodes too. Validators populate these via their own handlers. + // Skip invalid-proposal marking while the escape hatch is open, matching the validator path, + // which intentionally disables invalid-block slashing then. + if (result.reason === 'checkpoint_proposal_equivocation') { + this.markProposalEquivocation(slotNumber); + } else if ( + SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT.includes(result.reason) && + !(await this.epochCache.isEscapeHatchOpenAtSlot(slotNumber)) + ) { + this.markInvalidProposalSlot(slotNumber); + } this.log.warn( `Non-validator block proposal ${blockNumber} at slot ${slotNumber} failed processing with ${result.reason}`, { blockNumber: result.blockNumber, slotNumber, reason: result.reason }, @@ -270,6 +351,11 @@ export class ProposalHandler { p2pClient.registerBlockProposalHandler(blockHandler); + // p2p detects duplicate (equivocated) proposals without routing them through the handlers above, so mark + // the slot as equivocated here. This suppresses false-positive attested-to-invalid-proposal slashing on + // non-validator offense collectors. Validators overwrite this with their own richer handler. + p2pClient.registerDuplicateProposalCallback(info => this.markProposalEquivocation(info.slot)); + // All-nodes checkpoint proposal handler: validates, caches, and sets proposed checkpoint for pipelining. // Runs for all nodes (validators and non-validators). Validators get the cached result in the // validator-specific callback (attestToCheckpointProposal) which runs after this one. @@ -318,6 +404,12 @@ export class ProposalHandler { const result = await this.handleCheckpointProposal(proposal, proposalInfo); if (!result.isValid) { + // Track invalid checkpoint proposals so offense observers (the attested-invalid-proposal watcher) + // work on non-validator nodes too. This handler runs for all nodes; validators also mark via the + // failure callback below (idempotent). + if (SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[result.reason]) { + this.markInvalidProposalSlot(proposal.slotNumber); + } await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo); } else if (this.archiver) { const set = await this.setProposedCheckpoint(proposal); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 8a081f5dc86b..c7f6d01de08a 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -66,48 +66,18 @@ import { NodeKeystoreAdapter } from './key_store/node_keystore_adapter.js'; import { ValidatorMetrics } from './metrics.js'; import { type BlockProposalValidationFailureReason, - type CheckpointProposalValidationFailureReason, type CheckpointProposalValidationFailureResult, ProposalHandler, + SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT, + SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT, } from './proposal_handler.js'; // We maintain a set of proposers who have proposed invalid blocks. // Just cap the set to avoid unbounded growth. const MAX_PROPOSERS_OF_INVALID_BLOCKS = 1000; -const MAX_TRACKED_INVALID_PROPOSAL_SLOTS = 1000; const MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS = 1000; const MAX_TRACKED_BAD_ATTESTATIONS = 10_000; -// What errors from the block proposal handler result in slashing -const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ - 'state_mismatch', - 'failed_txs', - 'global_variables_mismatch', - 'invalid_proposal', - 'parent_block_wrong_slot', - 'in_hash_mismatch', -]; - -const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record = { - // enabled - ['invalid_fee_asset_price_modifier']: true, - ['checkpoint_header_mismatch']: true, - // These late mismatches should normally be caught by earlier checks, but if reached after validating the local - // checkpoint inputs, the proposer-signed payload disagrees with deterministic recomputation. - ['archive_mismatch']: true, - ['out_hash_mismatch']: true, - ['no_blocks_for_slot']: true, - ['too_many_blocks_in_checkpoint']: true, - ['checkpoint_validation_failed']: true, - ['last_block_archive_mismatch']: true, - - // disabled - ['invalid_signature']: false, - ['last_block_not_found']: false, - ['block_fetch_error']: false, - ['checkpoint_already_published']: false, -}; - /** * Validator Client */ @@ -131,11 +101,9 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private lastAttestedEpochByAttester: Map = new Map(); private proposersOfInvalidBlocks = FifoSet.withLimit(MAX_PROPOSERS_OF_INVALID_BLOCKS); - private slotsWithInvalidProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); private invalidCheckpointProposalOffenseKeys = FifoSet.withLimit(MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS); private oversizedProposalOffenseKeys = FifoSet.withLimit(MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS); private badAttestationOffenseKeys = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); - private slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); /** Tracks the last checkpoint proposal we attested to, to prevent equivocation. */ private lastAttestedProposal?: CheckpointProposalCore; @@ -364,11 +332,11 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } public hasProposalEquivocation(slotNumber: SlotNumber): boolean { - return this.slotsWithProposalEquivocation.has(slotNumber); + return this.proposalHandler.hasProposalEquivocation(slotNumber); } public hasInvalidProposals(slotNumber: SlotNumber): boolean { - return this.slotsWithInvalidProposals.has(slotNumber); + return this.proposalHandler.hasInvalidProposals(slotNumber); } public updateConfig(config: Partial) { @@ -771,8 +739,8 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return; } - this.markInvalidProposalSlot(proposal.slotNumber); - + // The slot is already marked invalid by the all-nodes checkpoint handler that invokes this callback, + // so we only emit the proposer slash event here. if (this.slashInvalidCheckpointProposal(proposal)) { this.log.info(`Detected invalid checkpoint proposal offense`, { ...proposalInfo, @@ -811,12 +779,15 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } private markInvalidProposalSlot(slotNumber: SlotNumber): void { - this.slotsWithInvalidProposals.add(slotNumber); + this.proposalHandler.markInvalidProposalSlot(slotNumber); } private handleCheckpointAttestation(attestation: CheckpointAttestation): void { const slotNumber = attestation.slotNumber; - if (!this.slotsWithInvalidProposals.has(slotNumber) || this.slotsWithProposalEquivocation.has(slotNumber)) { + if ( + !this.proposalHandler.hasInvalidProposals(slotNumber) || + this.proposalHandler.hasProposalEquivocation(slotNumber) + ) { return; } @@ -891,7 +862,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) */ private handleDuplicateProposal(info: DuplicateProposalInfo): void { const { slot, proposer, type } = info; - this.slotsWithProposalEquivocation.add(slot); + this.proposalHandler.markProposalEquivocation(slot); this.log.info(`Detected duplicate ${type} proposal offense from ${proposer.toString()} at slot ${slot}`, { proposer: proposer.toString(), diff --git a/yarn-project/world-state/src/native/ipc_world_state_instance.ts b/yarn-project/world-state/src/native/ipc_world_state_instance.ts index ba8bb56a7c4b..6790e4880fa1 100644 --- a/yarn-project/world-state/src/native/ipc_world_state_instance.ts +++ b/yarn-project/world-state/src/native/ipc_world_state_instance.ts @@ -548,6 +548,9 @@ export class IpcWorldState implements NativeWorldStateInstance { blocknumber: Number(b.blockNumber), blockstateref: blockStateRefToMap(b.blockStateRef as Map) as any, blockheaderhash: new Uint8Array(b.blockHeaderHash), + // Forwarded so the wsdb (IPC) sync path enforces the same archive-root divergence check as the napi path. + expectedarchiveroot: new Uint8Array(b.expectedArchiveRoot), + expectedpreviousarchiveroot: new Uint8Array(b.expectedPreviousArchiveRoot), paddednotehashes: b.paddedNoteHashes.map(l => new Uint8Array(l as Buffer)), paddedl1tol2messages: b.paddedL1ToL2Messages.map(l => new Uint8Array(l as Buffer)), paddednullifiers: b.paddedNullifiers.map(l => ({ @@ -558,6 +561,7 @@ export class IpcWorldState implements NativeWorldStateInstance { value: new Uint8Array((l as { slot: Buffer; value: Buffer }).value), })), }); + return convertStatusFull(resp.status) as WorldStateResponse[T]; } diff --git a/yarn-project/world-state/src/native/message.ts b/yarn-project/world-state/src/native/message.ts index 138e3605f007..26843cbeda2e 100644 --- a/yarn-project/world-state/src/native/message.ts +++ b/yarn-project/world-state/src/native/message.ts @@ -423,6 +423,10 @@ interface SyncBlockRequest extends WithCanonicalForkId { blockNumber: BlockNumber; blockStateRef: BlockStateReference; blockHeaderHash: Buffer; + /** Canonical archive root after this block; world state rejects the sync if its computed root differs. */ + expectedArchiveRoot: Buffer; + /** Canonical archive root before this block (the block's lastArchive); verified against the local root. */ + expectedPreviousArchiveRoot: Buffer; paddedNoteHashes: readonly SerializedLeafValue[]; paddedL1ToL2Messages: readonly SerializedLeafValue[]; paddedNullifiers: readonly SerializedLeafValue[]; diff --git a/yarn-project/world-state/src/native/native_world_state.test.ts b/yarn-project/world-state/src/native/native_world_state.test.ts index 414dcd0d2591..865c9fdfaff2 100644 --- a/yarn-project/world-state/src/native/native_world_state.test.ts +++ b/yarn-project/world-state/src/native/native_world_state.test.ts @@ -9,7 +9,7 @@ import { NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP, PUBLIC_DATA_TREE_HEIGHT, } from '@aztec/constants'; -import { BlockNumber } from '@aztec/foundation/branded-types'; +import { BlockNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { timesAsync } from '@aztec/foundation/collection'; import { randomBytes } from '@aztec/foundation/crypto/random'; import { Fr } from '@aztec/foundation/curves/bn254'; @@ -32,7 +32,7 @@ import { tmpdir } from 'os'; import { join } from 'path'; import type { WorldStateTreeMapSizes } from '../synchronizer/factory.js'; -import { assertSameState, compareChains, mockBlock, mockEmptyBlock } from '../test/utils.js'; +import { assertSameState, compareChains, mockBlock, mockEmptyBlock, updateBlockState } from '../test/utils.js'; import { INITIAL_NULLIFIER_TREE_SIZE, INITIAL_PUBLIC_DATA_TREE_SIZE } from '../world-state-db/merkle_tree_db.js'; import type { WorldStateStatusSummary } from './message.js'; import { NativeWorldStateService, WORLD_STATE_DB_VERSION, WORLD_STATE_DIR } from './native_world_state.js'; @@ -1043,6 +1043,93 @@ describe('NativeWorldState', () => { }); }); + describe('Archive root divergence on empty blocks', () => { + let ws: NativeWorldStateService; + + beforeEach(async () => { + ws = await NativeWorldStateService.new(EthAddress.random(), dataDir, wsTreeMapSizes); + }); + + afterEach(async () => { + await ws.close(); + }); + + const emptyMessages = () => Array(16).fill(0).map(Fr.zero); + + // A *different* empty block at height 1: the same (empty) contents as the canonical block, but a different slot, + // so a different block-header hash — the proposer-race orphan from A-1235, not a tampered block. + const buildDifferentBlockOne = async (slotNumber: number) => { + const fork = await ws.fork(); + const block = L2Block.empty(); + block.header.globalVariables.blockNumber = BlockNumber(1); + block.header.globalVariables.slotNumber = SlotNumber(slotNumber); + const messages = emptyMessages(); + await updateBlockState(block, messages, fork); + await fork.close(); + return { block, messages }; + }; + + // The canonical chain L1 finalized: block 1, then block 2 chained onto it (block 2's lastArchive == block 1's + // archive root). Built on a throwaway fork so it never touches the world state under test. + const buildCanonicalChain = async () => { + const fork = await ws.fork(); + const { block: canonicalOne } = await mockEmptyBlock(BlockNumber(1), fork); + const { block: canonicalTwo, messages: canonicalTwoMessages } = await mockEmptyBlock(BlockNumber(2), fork); + await fork.close(); + return { canonicalOne, canonicalTwo, canonicalTwoMessages }; + }; + + // The core blind spot: two empty blocks at the same height with different headers are indistinguishable to the + // four-tree state reference, yet they are different blocks with different archive roots. + it('two empty blocks at the same height with different headers share a state reference but not an archive root', async () => { + const { canonicalOne } = await buildCanonicalChain(); + const { block: orphanOne } = await buildDifferentBlockOne(99); + + // The four non-archive trees are identical (empty blocks insert no leaves), so is_same_state_reference cannot + // tell the two blocks apart... + expect(orphanOne.header.state).toEqual(canonicalOne.header.state); + // ...yet they are genuinely different blocks, with different header hashes and different archive roots. + expect((await orphanOne.hash()).equals(await canonicalOne.hash())).toBe(false); + expect(orphanOne.archive.root.equals(canonicalOne.archive.root)).toBe(false); + }); + + // The seeding step: a self-consistent orphan block passes every check, so world state takes the wrong block and + // ends up on the orphan fork — it has no way, from this block alone, to know it is not the canonical block 1. + it('silently accepts a different empty block at the same height (how the wrong block gets in)', async () => { + const { canonicalOne } = await buildCanonicalChain(); + const { block: orphanOne, messages: orphanMessages } = await buildDifferentBlockOne(99); + + await expect(ws.handleL2BlockAndMessages(orphanOne, orphanMessages)).resolves.toBeDefined(); + + // World state is now on the orphan fork: its committed archive root is the orphan's, not canonical block 1's. + const archive = await ws.getCommitted().getTreeInfo(MerkleTreeId.ARCHIVE); + expect(Fr.fromBuffer(archive.root).equals(orphanOne.archive.root)).toBe(true); + expect(Fr.fromBuffer(archive.root).equals(canonicalOne.archive.root)).toBe(false); + }); + + // The fix: once world state has taken the orphan, the canonical successor (which chains off the real block 1) + // no longer matches world state's committed archive root, and the pre-append guard rejects it before committing. + it('rejects the canonical successor of a wrongly-synced orphan, catching the archive divergence (the fix)', async () => { + const { canonicalTwo, canonicalTwoMessages } = await buildCanonicalChain(); + const { block: orphanOne, messages: orphanMessages } = await buildDifferentBlockOne(99); + + // The orphan commits cleanly (it is a self-consistent block 1). + await expect(ws.handleL2BlockAndMessages(orphanOne, orphanMessages)).resolves.toBeDefined(); + const archiveBefore = await ws.getCommitted().getTreeInfo(MerkleTreeId.ARCHIVE); + + // canonicalTwo.lastArchive == canonicalOne's archive root, which no longer matches world state's committed + // (orphan) archive root, so the pre-append guard throws before committing. + await expect(ws.handleL2BlockAndMessages(canonicalTwo, canonicalTwoMessages)).rejects.toThrow( + /diverged from the canonical chain/, + ); + + // Clean rollback: the archive tree is untouched. + const archiveAfter = await ws.getCommitted().getTreeInfo(MerkleTreeId.ARCHIVE); + expect(archiveAfter.root).toEqual(archiveBefore.root); + expect(archiveAfter.size).toEqual(archiveBefore.size); + }); + }); + describe('Finding leaves', () => { let block: L2Block; let messages: Fr[]; diff --git a/yarn-project/world-state/src/native/native_world_state.ts b/yarn-project/world-state/src/native/native_world_state.ts index 4c69360b551c..4253a04df6b0 100644 --- a/yarn-project/world-state/src/native/native_world_state.ts +++ b/yarn-project/world-state/src/native/native_world_state.ts @@ -294,6 +294,9 @@ export class NativeWorldStateService implements MerkleTreeDatabase { { blockNumber: l2Block.number, blockHeaderHash: (await l2Block.hash()).toBuffer(), + // Forwarded so the native sync verifies the archive root against canonical and rejects a divergent tree. + expectedArchiveRoot: l2Block.archive.root.toBuffer(), + expectedPreviousArchiveRoot: l2Block.header.lastArchive.root.toBuffer(), paddedL1ToL2Messages: paddedL1ToL2Messages.map(serializeLeaf), paddedNoteHashes: paddedNoteHashes.map(serializeLeaf), paddedNullifiers: paddedNullifiers.map(serializeLeaf), diff --git a/yarn-project/world-state/src/test/utils.ts b/yarn-project/world-state/src/test/utils.ts index 7a9a2fd3a9fd..1364155ec208 100644 --- a/yarn-project/world-state/src/test/utils.ts +++ b/yarn-project/world-state/src/test/utils.ts @@ -60,7 +60,16 @@ export async function updateBlockState(block: L2Block, l1ToL2Messages: Fr[], for await Promise.all([publicDataInsert, nullifierInsert, noteHashInsert, messageInsert]); const state = await fork.getStateReference(); - block.header = BlockHeader.from({ ...block.header, state }); + + // Capture the archive root *before* appending this block so the header's lastArchive chains off the previous block. + // sync_block now verifies lastArchive against the committed archive root, so a block carrying an unchained value + // (e.g. the random lastArchive from L2Block.random) would be rejected as a divergence from the canonical chain. + const previousArchive = await fork.getTreeInfo(MerkleTreeId.ARCHIVE); + block.header = BlockHeader.from({ + ...block.header, + state, + lastArchive: new AppendOnlyTreeSnapshot(Fr.fromBuffer(previousArchive.root), Number(previousArchive.size)), + }); await fork.updateArchive(block.header); const archiveState = await fork.getTreeInfo(MerkleTreeId.ARCHIVE);