Implement chain adapater to read on chain protocol configurations by turmelclem · Pull Request #3482 · IntersectMBO/mithril

turmelclem · 2026-08-17T09:48:42Z

Content

This PR includes modification to implement chain adapater to read on chain protocol configurations :

Implement on-chain reader for protocol configurations
move mechanism checking configuration consistency in model
add markers implementation of MithrilNetworkConfigurationProvider
Implement a protocol configuration builder (used by Signer and Aggregator)
Wire MarkersMithrilNetworkConfigurationProvider in the Signer
Wire MarkersMithrilNetworkConfigurationProvider instead of local impl for leader aggregator
- tests have been adapted to read protocol configurations from makers instead of aggregator configuration
Refactor usage of parameters for protocol-configuration commands

Pre-submit checklist

Branch
- Tests are provided (if possible)
- Crates versions are updated (if relevant)
- CHANGELOG file is updated (if relevant)
- Commit sequence broadly makes sense
- Key commits have useful messages
PR
- All check jobs of the CI have succeeded
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested
Documentation
- Update README file (if relevant)
- Update documentation website (if relevant)
- Add dev blog post (if relevant)
- Add ADR blog post or Dev ADR entry (if relevant)
- No new TODOs introduced

Relates to #3393

github-actions · 2026-08-17T10:11:06Z

Test Results

5 files 209 suites 58m 46s ⏱️
3 381 tests 3 381 ✅ 0 💤 0 ❌
11 256 runs 11 256 ✅ 0 💤 0 ❌

Results for commit 551848d.

♻️ This comment has been updated with latest results.

…igurations

…nsistency in model

…kConfigurationProvider

…figurationProvider instead of local impl for leader aggregator

…guration commands

Alenar · 2026-08-20T07:58:14Z

+    }
+
+    /// Verify the signature of a signed protocol configuration markers payload
+    pub fn verify_signature(


I think this method should be tested, at least with a roundtrip test (create a full message with at least two markers and a legitimate signature, then run this method).

Alenar · 2026-08-20T07:58:50Z

+use mithril_common::entities::Epoch;
+use std::collections::BTreeMap;
 use std::sync::Arc;

+use crate::cardano_chain::message::{
+    ProtocolConfigurationForEpochMessage, ProtocolConfigurationMarker,
+};
+use crate::cardano_chain::payload::SignedProtocolConfigurationMarkersPayload;
+use crate::interface::ProtocolConfigurationMarkersReader;
+use crate::model::{ConfigurationResolverFromMarkers, ProtocolConfigurationForEpoch};
 use mithril_cardano_node_chain::chain_observer::ChainObserver;
-use mithril_cardano_node_chain::entities::ChainAddress;
-use mithril_common::StdResult;
+use mithril_cardano_node_chain::entities::{ChainAddress, TxDatumFieldTypeName};
 use mithril_common::crypto_helper::ProtocolConfigurationMarkersVerifierVerificationKey;
-
-use crate::interface::ProtocolConfigurationMarkersReader;
-use crate::model::ConfigurationResolverFromMarkers;
+use mithril_common::{StdError, StdResult};


Some import issues here (epoch in the wrong block, use crate mixed up)

Alenar · 2026-08-20T08:05:07Z

+        let markers_list = tx_datums
+            .into_iter()
+            .filter_map(|datum| datum.get_fields_by_type(&TxDatumFieldTypeName::Bytes).ok())
+            .map(|fields| {
+                fields
+                    .iter()
+                    .filter_map(|field_value| field_value.as_str().map(|s| s.to_string()))
+                    .collect::<Vec<String>>()
+                    .join("")
+            })
+            .filter_map(|field_value_str| {
+                SignedProtocolConfigurationMarkersPayload::from_json_hex(&field_value_str).ok()
+            })
+            .filter_map(|markers_payload| {
+                markers_payload
+                    .verify_signature(self.verification_key)
+                    .ok()
+                    .map(|_| markers_payload.markers)
+            })
+            .collect::<Vec<Vec<ProtocolConfigurationMarker>>>();
+
+        let last_markers = markers_list.first().cloned().unwrap_or_default();


Wouldn't it be more efficient to only read the first item? Is there any reason to collecting them all?

Suggested change

let markers_list = tx_datums

.into_iter()

.filter_map(|datum| datum.get_fields_by_type(&TxDatumFieldTypeName::Bytes).ok())

.map(|fields| {

fields

.iter()

.filter_map(|field_value| field_value.as_str().map(|s| s.to_string()))

.collect::<Vec<String>>()

.join("")

})

.filter_map(|field_value_str| {

SignedProtocolConfigurationMarkersPayload::from_json_hex(&field_value_str).ok()

})

.filter_map(|markers_payload| {

markers_payload

.verify_signature(self.verification_key)

.ok()

.map(|_| markers_payload.markers)

})

.collect::<Vec<Vec<ProtocolConfigurationMarker>>>();

let last_markers = markers_list.first().cloned().unwrap_or_default();

let last_markers: Vec<ProtocolConfigurationMarker> = tx_datums

.into_iter()

.filter_map(|datum| datum.get_fields_by_type(&TxDatumFieldTypeName::Bytes).ok())

.map(|fields| {

fields

.iter()

.filter_map(|field_value| field_value.as_str().map(|s| s.to_string()))

.collect::<Vec<String>>()

.join("")

})

.filter_map(|field_value_str| {

SignedProtocolConfigurationMarkersPayload::from_json_hex(&field_value_str).ok()

})

.filter_map(|markers_payload| {

markers_payload

.verify_signature(self.verification_key)

.ok()

.map(|_| markers_payload.markers)

})

.next()

.unwrap_or_default();

Alenar · 2026-08-20T08:07:06Z

+                    .map_err(|e| {
+                        StdError::msg(format!(
+                            "ProtocolConfigurationForEpochMessage for Epoch({}) could not be decoded from cbor hex: {}",
+                            marker.epoch,
+                            e
+                        ))
+                    })?


Is there a some incompatibility with anyhow context for this convoluted map?

Suggested change

.map_err(|e| {

StdError::msg(format!(

"ProtocolConfigurationForEpochMessage for Epoch({}) could not be decoded from cbor hex: {}",

marker.epoch,

e

))

})?

.with_context(|| {

format!(

"ProtocolConfigurationForEpochMessage for Epoch({}) could not be decoded from cbor hex",

marker.epoch,

)

})?

Alenar · 2026-08-20T08:09:49Z

+    }
+
+    #[tokio::test]
+    async fn test_cardano_chain_reader() {


The test name is too broad, here is a tentative name:

Suggested change

async fn test_cardano_chain_reader() {

async fn markers_signed_by_third_parties_are_rejected() {

Alenar · 2026-08-20T08:24:46Z

+use mithril_cli_helper::{register_config_value, serde_deserialization};
 use mithril_dmq::DmqNetwork;
 use mithril_doc::{Documenter, DocumenterDefault, StructDoc};
+use mithril_protocol_config::builder::AdapterConfig;


You could use this opportunity to move those mithril_* uses below

Alenar · 2026-08-20T08:27:01Z

+}
+pub mod protocol_configuration {
+    use super::*;


Oups, missing line breaks

Suggested change

}

pub mod protocol_configuration {

use super::*;

}

pub mod protocol_configuration {

use super::*;

Alenar · 2026-08-20T08:42:38Z

-            self.root_logger(),
-        ));
+
+        let protocol_configuration_adapter = build_protocol_configuration_adapter(


I wonder if we should adapt the signer integration tests as well?

Alenar · 2026-08-20T09:46:57Z

        cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
            security_parameter: BlockNumberOffset(0),
            step: BlockNumber(30),
        }),
        cardano_blocks_transactions_signing_config: Some(CardanoBlocksTransactionsSigningConfig {
            security_parameter: BlockNumberOffset(0),
            step: BlockNumber(24),
        }),


Maybe you should remove those configurations?

Alenar · 2026-08-20T09:48:33Z


 use anyhow::Context;
 use config::{ConfigError, Map, Source, Value, ValueKind};
+use mithril_protocol_config::builder::AdapterConfig;


this mithril import should be below

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from 4f87a71 to cb72f72 Compare August 17, 2026 09:56

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from cb72f72 to 9a7ac9a Compare August 17, 2026 16:10

github-advanced-security Bot found potential problems Aug 17, 2026

View reviewed changes

turmelclem self-assigned this Aug 18, 2026

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from 9a7ac9a to 4a79c83 Compare August 18, 2026 16:36

github-advanced-security Bot found potential problems Aug 18, 2026

View reviewed changes

Comment thread mithril-aggregator/tests/genesis_to_signing.rs Fixed

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from 4a79c83 to 192a144 Compare August 19, 2026 09:59

github-advanced-security Bot found potential problems Aug 19, 2026

View reviewed changes

Comment thread mithril-signer/src/configuration.rs Fixed

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from 192a144 to 38c6548 Compare August 19, 2026 12:25

turmelclem marked this pull request as ready for review August 19, 2026 13:06

turmelclem requested review from Alenar and jpraynaud as code owners August 19, 2026 13:06

turmelclem added 7 commits August 19, 2026 15:09

feature(protocol-config): Implement on-chain reader for protocol conf…

b120bff

…igurations

refactor(protocol-config): moving mechanism checking configuration co…

4d11d17

…nsistency in model

feature(protocol-config): add markers implementation of MithrilNetwor…

dcc3a65

…kConfigurationProvider

feature(protocol-config): Implement a protocol configuration builder

6b697b1

feature(signer): Wiring MarkersMithrilNetworkConfigurationProvider

0a85b50

feature(aggregator, protocol-config): Wiring MarkersMithrilNetworkCon…

b04f085

…figurationProvider instead of local impl for leader aggregator

refactor(aggregator): refactor usage of parameters for protocol-confi…

551848d

…guration commands

turmelclem force-pushed the ctl/3393-implement-chain-adapater-to-read-on-chain-protocol-configurations-2 branch from 38c6548 to 551848d Compare August 19, 2026 13:09

Alenar reviewed Aug 20, 2026

View reviewed changes

	async fn test_cardano_chain_reader() {
	async fn markers_signed_by_third_parties_are_rejected() {

Uh oh!

Conversation

turmelclem commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Pre-submit checklist

Uh oh!

github-actions Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

turmelclem commented Aug 17, 2026 •

edited

Loading

github-actions Bot commented Aug 17, 2026 •

edited

Loading