diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md index cb2ff0b..03b8663 100644 --- a/BREAKING_CHANGES.md +++ b/BREAKING_CHANGES.md @@ -7,11 +7,26 @@ Each bullet is prefixed with a flag identifying the kind of breaking change: - `[CLI]` -- CLI flag added, renamed, removed, or made required. - `[Config]` -- default value, environment variable, or manifest field change. - `[Format]` -- log, metric label, or serialized output format change that breaks parsers. +- `[API]` -- public Rust API change that can break downstream crates. Entries are split by audience. A change appears under `### For Validators` when validator-mode operation must change; otherwise it appears under `### For Node Operators`. A change requiring both audiences to act appears in both sections (rare). +Rust API compatibility changes appear under `### For Rust API Consumers`. Compare and release-notes links resolve once the corresponding tag is published at [`circlefin/arc-node`](https://github.com/circlefin/arc-node). +## [Unreleased] + +### For Rust API Consumers + +- **[API] `ArcConsensusBuilder` is no longer `Clone` or `Copy`.** + - Old (`v0.7.2`): `ArcConsensusBuilder` derived `Clone` and `Copy`, so + downstream Rust code could duplicate the builder after construction. + - New (next release): the builder can hold a `Box` + consensus modifier and is therefore move-only. + - Code that cloned or copied the builder must construct a fresh + `ArcConsensusBuilder::default()` or finish applying builder configuration + before moving it into the node builder. + ## [v0.7.2] **Changes:** [v0.7.1...v0.7.2](https://github.com/circlefin/arc-node/compare/v0.7.1...v0.7.2) -- [release notes](https://github.com/circlefin/arc-node/releases/tag/v0.7.2) diff --git a/CHANGELOG.md b/CHANGELOG.md index 515b699..c8a33cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to arc-node are documented in this file. +## [Unreleased] + +### Breaking Changes + +- **[API] `ArcConsensusBuilder` is no longer `Clone` or `Copy`.** The builder + can now hold a one-shot consensus modifier closure, so downstream Rust code + that cloned or copied the builder must construct a fresh builder or finish + configuring it before moving it into the node builder. See + [BREAKING_CHANGES.md](./BREAKING_CHANGES.md#unreleased) for migration + details. + ## [v0.7.2] **Changes:** [v0.7.1...v0.7.2](https://github.com/circlefin/arc-node/compare/v0.7.1...v0.7.2) -- [release notes](https://github.com/circlefin/arc-node/releases/tag/v0.7.2) diff --git a/crates/evm-node/src/node.rs b/crates/evm-node/src/node.rs index 8bb65fc..94855fc 100644 --- a/crates/evm-node/src/node.rs +++ b/crates/evm-node/src/node.rs @@ -647,10 +647,57 @@ where } } +type ArcConsensusModifier = Box< + dyn FnOnce(Arc>) -> eyre::Result>> + + Send + + 'static, +>; + /// A basic Arc consensus builder. -#[derive(Debug, Default, Clone, Copy)] +#[derive(Default)] pub struct ArcConsensusBuilder { - // TODO add closure to modify consensus + modify_consensus: Option, +} + +impl std::fmt::Debug for ArcConsensusBuilder { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ArcConsensusBuilder") + .field("modify_consensus", &self.modify_consensus.is_some()) + .finish() + } +} + +impl ArcConsensusBuilder { + /// Returns a builder that applies `modify_consensus` after constructing the + /// default [`ArcConsensus`]. + /// + /// Calling this more than once replaces the previous modifier. To apply + /// multiple transformations, compose them into one closure before passing it + /// to this method. + pub fn with_consensus_modifier(mut self, modify_consensus: F) -> Self + where + F: FnOnce(Arc>) -> eyre::Result>> + + Send + + 'static, + { + debug_assert!( + self.modify_consensus.is_none(), + "consensus modifier already set; compose modifiers before passing them to ArcConsensusBuilder" + ); + self.modify_consensus = Some(Box::new(modify_consensus)); + self + } + + /// Applies the configured modifier to a constructed consensus instance. + pub(crate) fn apply_consensus_modifier( + self, + consensus: Arc>, + ) -> eyre::Result>> { + match self.modify_consensus { + Some(modify_consensus) => modify_consensus(consensus), + None => Ok(consensus), + } + } } impl ConsensusBuilder for ArcConsensusBuilder @@ -660,7 +707,7 @@ where type Consensus = Arc::ChainSpec>>; async fn build_consensus(self, ctx: &BuilderContext) -> eyre::Result { - Ok(Arc::new(ArcConsensus::new(ctx.chain_spec()))) + self.apply_consensus_modifier(Arc::new(ArcConsensus::new(ctx.chain_spec()))) } } @@ -869,4 +916,35 @@ mod tests { ArcNetworkBuilder::default().with_rebroadcast_interval(std::time::Duration::ZERO); assert!(builder.rebroadcast_interval.is_zero()); } + + #[test] + fn arc_consensus_builder_applies_modifier() { + use std::sync::atomic::{AtomicBool, Ordering}; + + let called = Arc::new(AtomicBool::new(false)); + let called_for_modifier = called.clone(); + let consensus = Arc::new(ArcConsensus::new( + arc_execution_config::chainspec::LOCAL_DEV.clone(), + )); + + let modified = ArcConsensusBuilder::default() + .with_consensus_modifier(move |consensus| { + called_for_modifier.store(true, Ordering::Relaxed); + Ok(consensus) + }) + .apply_consensus_modifier(consensus.clone()) + .expect("modifier should succeed"); + + assert!(called.load(Ordering::Relaxed)); + assert!(Arc::ptr_eq(&modified, &consensus)); + } + + #[test] + #[cfg(debug_assertions)] + #[should_panic(expected = "consensus modifier already set")] + fn arc_consensus_builder_panics_on_double_modifier_in_debug() { + let _builder = ArcConsensusBuilder::default() + .with_consensus_modifier(Ok) + .with_consensus_modifier(Ok); + } }