fix(consensus): payload encoding protected by protocol_version

.github/workflows/ci-core-reusable.yml

@@ -8,6 +8,9 @@ on:
         required: false
         default: '[{ "zksolc": ["1.3.14", "1.3.16", "1.3.17", "1.3.1", "1.3.7", "1.3.18", "1.3.19", "1.3.21"] } , { "zkvyper": ["1.3.13"] }]'
 
+env:
+  RUST_BACKTRACE: 1
+
 jobs:
   lint:
     name: lint

core/lib/dal/src/consensus/conv.rs

@@ -2,7 +2,7 @@
 use anyhow::{anyhow, Context as _};
 use zksync_concurrency::net;
 use zksync_consensus_roles::{attester, node};
-use zksync_protobuf::{read_required, required, ProtoFmt, ProtoRepr};
+use zksync_protobuf::{read_optional_repr, read_required, required, ProtoFmt, ProtoRepr};
 use zksync_types::{
     abi,
     commitment::{L1BatchCommitmentMode, PubdataParams},
@@ -104,6 +104,31 @@ impl ProtoFmt for AttestationStatus {
     }
 }
 
+impl ProtoRepr for proto::PubdataParams {
+    type Type = PubdataParams;
+
+    fn read(&self) -> anyhow::Result<Self::Type> {
+        Ok(Self::Type {
+            l2_da_validator_address: required(&self.l2_da_validator_address)
+                .and_then(|a| parse_h160(a))
+                .context("l2_da_validator_address")?,
+            pubdata_type: required(&self.pubdata_type)
+                .and_then(|x| Ok(proto::L1BatchCommitDataGeneratorMode::try_from(*x)?))
+                .context("pubdata_type")?
+                .parse(),
+        })
+    }
+
+    fn build(this: &Self::Type) -> Self {
+        Self {
+            l2_da_validator_address: Some(this.l2_da_validator_address.as_bytes().into()),
+            pubdata_type: Some(
+                proto::L1BatchCommitDataGeneratorMode::new(&this.pubdata_type) as i32,
+            ),
+        }
+    }
+}
+
 impl ProtoFmt for Payload {
     type Proto = proto::Payload;
 
@@ -137,21 +162,7 @@ impl ProtoFmt for Payload {
             }
         }
 
-        let pubdata_params = if let Some(pubdata_params) = &r.pubdata_params {
-            Some(PubdataParams {
-                l2_da_validator_address: required(&pubdata_params.l2_da_validator_address)
-                    .and_then(|a| parse_h160(a))
-                    .context("l2_da_validator_address")?,
-                pubdata_type: required(&pubdata_params.pubdata_type)
-                    .and_then(|x| Ok(proto::L1BatchCommitDataGeneratorMode::try_from(*x)?))
-                    .context("pubdata_type")?
-                    .parse(),
-            })
-        } else {
-            None
-        };
-
-        Ok(Self {
+        let this = Self {
             protocol_version,
             hash: required(&r.hash)
                 .and_then(|h| parse_h256(h))
@@ -169,11 +180,17 @@ impl ProtoFmt for Payload {
                 .context("operator_address")?,
             transactions,
             last_in_batch: *required(&r.last_in_batch).context("last_in_batch")?,
-            pubdata_params,
-        })
+            pubdata_params: read_optional_repr(&r.pubdata_params).context("pubdata_params")?,
+        };
+        anyhow::ensure!(this.pubdata_params.is_none() == this.protocol_version.is_pre_gateway());
+        Ok(this)
     }
 
     fn build(&self) -> Self::Proto {
+        assert!(
+            self.protocol_version.is_pre_gateway() == self.pubdata_params.is_none(),
+            "pubdata_params should be None iff protocol_version is pre-gateway"
+        );
         let mut x = Self::Proto {
             protocol_version: Some((self.protocol_version as u16).into()),
             hash: Some(self.hash.as_bytes().into()),
@@ -188,16 +205,7 @@ impl ProtoFmt for Payload {
             transactions: vec![],
             transactions_v25: vec![],
             last_in_batch: Some(self.last_in_batch),
-            pubdata_params: self
-                .pubdata_params
-                .map(|pubdata_params| proto::PubdataParams {
-                    l2_da_validator_address: Some(
-                        pubdata_params.l2_da_validator_address.as_bytes().into(),
-                    ),
-                    pubdata_type: Some(proto::L1BatchCommitDataGeneratorMode::new(
-                        &pubdata_params.pubdata_type,
-                    ) as i32),
-                }),
+            pubdata_params: self.pubdata_params.as_ref().map(ProtoRepr::build),
         };
         match self.protocol_version {
             v if v >= ProtocolVersionId::Version25 => {

core/lib/dal/src/consensus/tests.rs

@@ -1,7 +1,7 @@
 use std::fmt::Debug;
 
 use rand::Rng;
-use zksync_concurrency::ctx;
+use zksync_concurrency::{ctx, testonly::abort_on_panic};
 use zksync_protobuf::{
     repr::{decode, encode},
     testonly::{test_encode, test_encode_all_formats, FmtConv},
@@ -53,19 +53,24 @@ fn payload(rng: &mut impl Rng, protocol_version: ProtocolVersionId) -> Payload {
             })
             .collect(),
         last_in_batch: rng.gen(),
-        pubdata_params: Some(PubdataParams {
-            pubdata_type: match rng.gen_range(0..2) {
-                0 => L1BatchCommitmentMode::Rollup,
-                _ => L1BatchCommitmentMode::Validium,
-            },
-            l2_da_validator_address: rng.gen(),
-        }),
+        pubdata_params: if !protocol_version.is_pre_gateway() {
+            Some(PubdataParams {
+                pubdata_type: match rng.gen_range(0..2) {
+                    0 => L1BatchCommitmentMode::Rollup,
+                    _ => L1BatchCommitmentMode::Validium,
+                },
+                l2_da_validator_address: rng.gen(),
+            })
+        } else {
+            None
+        },
     }
 }
 
 /// Tests struct <-> proto struct conversions.
 #[test]
 fn test_encoding() {
+    abort_on_panic();
     let ctx = &ctx::test_root(&ctx::RealClock);
     let rng = &mut ctx.rng();
     test_encode_all_formats::<FmtConv<AttestationStatus>>(rng);
@@ -78,10 +83,15 @@ fn test_encoding() {
     encode_decode::<proto::Transaction, ComparableTransaction>(
         mock_protocol_upgrade_transaction().into(),
     );
-    let p = payload(rng, ProtocolVersionId::Version24);
-    test_encode(rng, &p);
-    let p = payload(rng, ProtocolVersionId::Version25);
-    test_encode(rng, &p);
+    // Test encoding in the current and all the future versions.
+    for v in ProtocolVersionId::latest() as u16.. {
+        let Ok(v) = ProtocolVersionId::try_from(v) else {
+            break;
+        };
+        tracing::info!("version {v}");
+        let p = payload(rng, v);
+        test_encode(rng, &p);
+    }
 }
 
 fn encode_decode<P, C>(msg: P::Type)

core/lib/dal/src/models/storage_sync.rs

@@ -139,7 +139,11 @@ impl SyncBlock {
             operator_address: self.fee_account_address,
             transactions,
             last_in_batch: self.last_in_batch,
-            pubdata_params: Some(self.pubdata_params),
+            pubdata_params: if self.protocol_version.is_pre_gateway() {
+                Some(self.pubdata_params)
+            } else {
+                None
+            },
         }
     }
 }