feat: CON-1422 CON-1423 Introduce VetKD payload section and deliver c…

…ompleted agreements (#4022) This PR introduces a new VetKD payload section that is filled and validated by the new `VetKdPayloadBuilder`. The section contains completed VetKD agreements, which are deserialized and delivered to execution as part of the `Batch`. The protobuf change is compatible with the mainnet version, as it only affects payloads of data blocks (as opposed to summary blocks, which do need to be compatible during upgrades). --------- Co-authored-by: IDX GitHub Automation <[email protected]>
dfinity · Mar 3, 2025 · 33fd3f2 · 33fd3f2
1 parent ea73f6e
commit 33fd3f2
Show file tree

Hide file tree

Showing 21 changed files with 157 additions and 1 deletion.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/rs/consensus/BUILD.bazel b/rs/consensus/BUILD.bazel
@@ -9,6 +9,7 @@ DEPENDENCIES = [
     "//rs/config",
     "//rs/consensus/dkg",
     "//rs/consensus/utils",
+    "//rs/consensus/vetkd",
     "//rs/crypto",
     "//rs/crypto/prng",
     "//rs/crypto/utils/threshold_sig_der",

diff --git a/rs/consensus/Cargo.toml b/rs/consensus/Cargo.toml
@@ -10,6 +10,7 @@ documentation.workspace = true
 ic-config = { path = "../config" }
 ic-consensus-dkg = { path = "./dkg" }
 ic-consensus-utils = { path = "./utils" }
+ic-consensus-vetkd = { path = "./vetkd" }
 ic-crypto = { path = "../crypto" }
 ic-crypto-prng = { path = "../crypto/prng" }
 ic-crypto-test-utils-canister-threshold-sigs = { path = "../crypto/test_utils/canister_threshold_sigs" }

diff --git a/rs/consensus/benches/validate_payload.rs b/rs/consensus/benches/validate_payload.rs
@@ -170,6 +170,7 @@ where
             Arc::new(FakeSelfValidatingPayloadBuilder::new()),
             Arc::new(FakeCanisterHttpPayloadBuilder::new()),
             Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
+            Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
             metrics_registry,
             no_op_logger(),
         ));

diff --git a/rs/consensus/src/consensus.rs b/rs/consensus/src/consensus.rs
@@ -143,6 +143,7 @@ impl ConsensusImpl {
         self_validating_payload_builder: Arc<dyn SelfValidatingPayloadBuilder>,
         canister_http_payload_builder: Arc<dyn BatchPayloadBuilder>,
         query_stats_payload_builder: Arc<dyn BatchPayloadBuilder>,
+        vetkd_payload_builder: Arc<dyn BatchPayloadBuilder>,
         dkg_pool: Arc<RwLock<dyn DkgPool>>,
         idkg_pool: Arc<RwLock<dyn IDkgPool>>,
         dkg_key_manager: Arc<Mutex<DkgKeyManager>>,
@@ -171,6 +172,7 @@ impl ConsensusImpl {
             self_validating_payload_builder,
             canister_http_payload_builder,
             query_stats_payload_builder,
+            vetkd_payload_builder,
             metrics_registry.clone(),
             logger.clone(),
         ));
@@ -682,6 +684,7 @@ mod tests {
             Arc::new(FakeSelfValidatingPayloadBuilder::new()),
             Arc::new(FakeCanisterHttpPayloadBuilder::new()),
             Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
+            Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
             dkg_pool,
             idkg_pool,
             Arc::new(Mutex::new(DkgKeyManager::new(

diff --git a/rs/consensus/src/consensus/batch_delivery.rs b/rs/consensus/src/consensus/batch_delivery.rs
@@ -13,6 +13,7 @@ use ic_consensus_dkg::get_vetkey_public_keys;
 use ic_consensus_utils::{
     crypto_hashable_to_seed, membership::Membership, pool_reader::PoolReader,
 };
+use ic_consensus_vetkd::VetKdPayloadBuilderImpl;
 use ic_https_outcalls_consensus::payload_builder::CanisterHttpPayloadBuilderImpl;
 use ic_interfaces::{
     batch_payload::IntoMessages,
@@ -285,6 +286,10 @@ pub fn generate_responses_to_subnet_calls(
             CanisterHttpPayloadBuilderImpl::into_messages(&block_payload.batch.canister_http);
         consensus_responses.append(&mut http_responses);
         stats.canister_http = http_stats;
+
+        let mut vetkd_responses =
+            VetKdPayloadBuilderImpl::into_messages(&block_payload.batch.vetkd);
+        consensus_responses.append(&mut vetkd_responses);
     }
     consensus_responses
 }

diff --git a/rs/consensus/src/consensus/payload.rs b/rs/consensus/src/consensus/payload.rs
@@ -43,6 +43,7 @@ pub(crate) enum BatchPayloadSectionBuilder {
     SelfValidating(Arc<dyn SelfValidatingPayloadBuilder>),
     CanisterHttp(Arc<dyn BatchPayloadBuilder>),
     QueryStats(Arc<dyn BatchPayloadBuilder>),
+    VetKd(Arc<dyn BatchPayloadBuilder>),
 }
 
 impl BatchPayloadSectionBuilder {
@@ -92,6 +93,7 @@ impl BatchPayloadSectionBuilder {
             Self::SelfValidating(_) => "self_validating",
             Self::CanisterHttp(_) => "canister_http",
             Self::QueryStats(_) => "query_stats",
+            Self::VetKd(_) => "vetkd",
         }
     }
 
@@ -320,6 +322,44 @@ impl BatchPayloadSectionBuilder {
                     }
                 }
             }
+            Self::VetKd(builder) => {
+                let past_payloads: Vec<PastPayload> =
+                    filter_past_payloads(past_payloads, |_, _, payload| {
+                        if payload.is_summary() {
+                            None
+                        } else {
+                            Some(&payload.as_ref().as_data().batch.vetkd)
+                        }
+                    });
+
+                let vetkd = builder.build_payload(
+                    height,
+                    max_size,
+                    &past_payloads,
+                    proposal_context.validation_context,
+                );
+                let size = NumBytes::new(vetkd.len() as u64);
+
+                // Check validation as safety measure
+                match builder.validate_payload(height, proposal_context, &vetkd, &past_payloads) {
+                    Ok(()) => {
+                        payload.vetkd = vetkd;
+                        size
+                    }
+                    Err(err) => {
+                        error!(
+                            logger,
+                            "VetKd payload did not pass validation, this is a bug, {:?} @{}",
+                            err,
+                            CRITICAL_ERROR_VALIDATION_NOT_PASSED
+                        );
+
+                        metrics.critical_error_validation_not_passed.inc();
+                        payload.vetkd = vec![];
+                        NumBytes::new(0)
+                    }
+                }
+            }
         }
     }
 
@@ -405,6 +445,25 @@ impl BatchPayloadSectionBuilder {
 
                 Ok(NumBytes::new(payload.query_stats.len() as u64))
             }
+            Self::VetKd(builder) => {
+                let past_payloads: Vec<PastPayload> =
+                    filter_past_payloads(past_payloads, |_, _, payload| {
+                        if payload.is_summary() {
+                            None
+                        } else {
+                            Some(&payload.as_ref().as_data().batch.vetkd)
+                        }
+                    });
+
+                builder.validate_payload(
+                    height,
+                    proposal_context,
+                    &payload.vetkd,
+                    &past_payloads,
+                )?;
+
+                Ok(NumBytes::new(payload.vetkd.len() as u64))
+            }
         }
     }
 }

diff --git a/rs/consensus/src/consensus/payload_builder.rs b/rs/consensus/src/consensus/payload_builder.rs
@@ -49,6 +49,7 @@ impl PayloadBuilderImpl {
         self_validating_payload_builder: Arc<dyn SelfValidatingPayloadBuilder>,
         canister_http_payload_builder: Arc<dyn BatchPayloadBuilder>,
         query_stats_payload_builder: Arc<dyn BatchPayloadBuilder>,
+        vetkd_payload_builder: Arc<dyn BatchPayloadBuilder>,
         metrics: MetricsRegistry,
         logger: ReplicaLogger,
     ) -> Self {
@@ -58,6 +59,7 @@ impl PayloadBuilderImpl {
             BatchPayloadSectionBuilder::XNet(xnet_payload_builder),
             BatchPayloadSectionBuilder::CanisterHttp(canister_http_payload_builder),
             BatchPayloadSectionBuilder::QueryStats(query_stats_payload_builder),
+            BatchPayloadSectionBuilder::VetKd(vetkd_payload_builder),
         ];
 
         Self {
@@ -272,6 +274,7 @@ pub(crate) mod test {
         let canister_http_payload_builder =
             FakeCanisterHttpPayloadBuilder::new().with_responses(canister_http_responses);
         let query_stats_payload_builder = MockBatchPayloadBuilder::new().expect_noop();
+        let vetkd_payload_builder = MockBatchPayloadBuilder::new().expect_noop();
 
         PayloadBuilderImpl::new(
             subnet_test_id(0),
@@ -282,6 +285,7 @@ pub(crate) mod test {
             Arc::new(self_validating_payload_builder),
             Arc::new(canister_http_payload_builder),
             Arc::new(query_stats_payload_builder),
+            Arc::new(vetkd_payload_builder),
             MetricsRegistry::new(),
             no_op_logger(),
         )

diff --git a/rs/consensus/tests/framework/runner.rs b/rs/consensus/tests/framework/runner.rs
@@ -160,6 +160,7 @@ impl<'a> ConsensusRunner<'a> {
             deps.self_validating_payload_builder.clone(),
             deps.canister_http_payload_builder.clone(),
             deps.query_stats_payload_builder.clone(),
+            deps.vetkd_payload_builder.clone(),
             deps.dkg_pool.clone(),
             deps.idkg_pool.clone(),
             dkg_key_manager.clone(),

diff --git a/rs/consensus/tests/framework/types.rs b/rs/consensus/tests/framework/types.rs
@@ -170,6 +170,7 @@ pub struct ConsensusDependencies {
     pub(crate) self_validating_payload_builder: Arc<dyn SelfValidatingPayloadBuilder>,
     pub(crate) canister_http_payload_builder: Arc<dyn BatchPayloadBuilder>,
     pub(crate) query_stats_payload_builder: Arc<dyn BatchPayloadBuilder>,
+    pub(crate) vetkd_payload_builder: Arc<dyn BatchPayloadBuilder>,
     pub consensus_pool: Arc<RwLock<ConsensusPoolImpl>>,
     pub dkg_pool: Arc<RwLock<dkg_pool::DkgPoolImpl>>,
     pub idkg_pool: Arc<RwLock<idkg_pool::IDkgPoolImpl>>,
@@ -228,6 +229,7 @@ impl ConsensusDependencies {
             self_validating_payload_builder: Arc::new(FakeSelfValidatingPayloadBuilder::new()),
             canister_http_payload_builder: Arc::new(FakeCanisterHttpPayloadBuilder::new()),
             query_stats_payload_builder: Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
+            vetkd_payload_builder: Arc::new(MockBatchPayloadBuilder::new().expect_noop()),
             state_manager,
             metrics_registry,
             replica_config,

diff --git a/rs/consensus/tests/payload.rs b/rs/consensus/tests/payload.rs
@@ -62,6 +62,9 @@ fn consensus_produces_expected_batches() {
         let query_stats_payload_builder = MockBatchPayloadBuilder::new().expect_noop();
         let query_stats_payload_builder = Arc::new(query_stats_payload_builder);
 
+        let vetkd_payload_builder = MockBatchPayloadBuilder::new().expect_noop();
+        let vetkd_payload_builder = Arc::new(vetkd_payload_builder);
+
         let mut state_manager = MockStateManager::new();
         state_manager.expect_remove_states_below().return_const(());
         state_manager
@@ -157,6 +160,7 @@ fn consensus_produces_expected_batches() {
             Arc::clone(&self_validating_payload_builder) as Arc<_>,
             Arc::clone(&canister_http_payload_builder) as Arc<_>,
             query_stats_payload_builder,
+            vetkd_payload_builder,
             Arc::clone(&dkg_pool) as Arc<_>,
             Arc::clone(&idkg_pool) as Arc<_>,
             dkg_key_manager.clone(),

diff --git a/rs/consensus/vetkd/src/lib.rs b/rs/consensus/vetkd/src/lib.rs
@@ -348,6 +348,8 @@ impl VetKdPayloadBuilderImpl {
                     invalid_artifact(InvalidVetKdPayloadReason::VetKdKeyVerificationError(err))
                 } else {
                     warn!(self.log, "VetKD payload validation failure: {err:?}");
+                    self.metrics
+                        .payload_errors_inc("validation_failed", &context.key_id());
                     validation_failed(VetKdPayloadValidationFailure::VetKdKeyVerificationError(
                         err,
                     ))

diff --git a/rs/protobuf/def/types/v1/consensus.proto b/rs/protobuf/def/types/v1/consensus.proto
@@ -67,6 +67,7 @@ message Block {
   reserved 14;
   bytes canister_http_payload_bytes = 15;
   bytes query_stats_payload_bytes = 16;
+  bytes vetkd_payload_bytes = 17;
   bytes payload_hash = 11;
 }
 

diff --git a/rs/protobuf/src/gen/types/types.v1.rs b/rs/protobuf/src/gen/types/types.v1.rs
@@ -1292,6 +1292,8 @@ pub struct Block {
     pub canister_http_payload_bytes: ::prost::alloc::vec::Vec<u8>,
     #[prost(bytes = "vec", tag = "16")]
     pub query_stats_payload_bytes: ::prost::alloc::vec::Vec<u8>,
+    #[prost(bytes = "vec", tag = "17")]
+    pub vetkd_payload_bytes: ::prost::alloc::vec::Vec<u8>,
     #[prost(bytes = "vec", tag = "11")]
     pub payload_hash: ::prost::alloc::vec::Vec<u8>,
 }

diff --git a/rs/replica/setup_ic_network/BUILD.bazel b/rs/replica/setup_ic_network/BUILD.bazel
@@ -8,6 +8,7 @@ DEPENDENCIES = [
     "//rs/config",
     "//rs/consensus/dkg",
     "//rs/consensus/utils",
+    "//rs/consensus/vetkd",
     "//rs/crypto/interfaces/sig_verification",
     "//rs/crypto/tls_interfaces",
     "//rs/cycles_account_manager",

diff --git a/rs/replica/setup_ic_network/Cargo.toml b/rs/replica/setup_ic_network/Cargo.toml
@@ -15,6 +15,7 @@ ic-consensus = { path = "../../consensus" }
 ic-consensus-dkg = { path = "../../consensus/dkg" }
 ic-consensus-manager = { path = "../../p2p/consensus_manager" }
 ic-consensus-utils = { path = "../../consensus/utils" }
+ic-consensus-vetkd = { path = "../../consensus/vetkd" }
 ic-crypto-interfaces-sig-verification = { path = "../../crypto/interfaces/sig_verification" }
 ic-crypto-tls-interfaces = { path = "../../crypto/tls_interfaces" }
 ic-cycles-account-manager = { path = "../../cycles_account_manager" }

diff --git a/rs/replica/setup_ic_network/src/lib.rs b/rs/replica/setup_ic_network/src/lib.rs
@@ -18,6 +18,7 @@ use ic_consensus::{
 use ic_consensus_dkg::DkgBouncer;
 use ic_consensus_manager::{AbortableBroadcastChannel, AbortableBroadcastChannelBuilder};
 use ic_consensus_utils::{crypto::ConsensusCrypto, pool_reader::PoolReader};
+use ic_consensus_vetkd::VetKdPayloadBuilderImpl;
 use ic_crypto_interfaces_sig_verification::IngressSigVerifier;
 use ic_crypto_tls_interfaces::TlsConfig;
 use ic_cycles_account_manager::CyclesAccountManager;
@@ -534,6 +535,17 @@ fn start_consensus(
         metrics_registry,
         log.clone(),
     ));
+
+    let vetkd_payload_builder = Arc::new(VetKdPayloadBuilderImpl::new(
+        artifact_pools.idkg_pool.clone(),
+        consensus_pool_cache.clone(),
+        consensus_crypto.clone(),
+        state_reader.clone(),
+        subnet_id,
+        registry_client.clone(),
+        metrics_registry,
+        log.clone(),
+    ));
     // ------------------------------------------------------------------------
 
     let replica_config = ReplicaConfig { node_id, subnet_id };
@@ -556,6 +568,7 @@ fn start_consensus(
         self_validating_payload_builder,
         https_outcalls_payload_builder,
         Arc::from(query_stats_payload_builder),
+        vetkd_payload_builder,
         Arc::clone(&artifact_pools.dkg_pool) as Arc<_>,
         Arc::clone(&artifact_pools.idkg_pool) as Arc<_>,
         Arc::clone(&dkg_key_manager) as Arc<_>,