refactor!(consensus): standardize dkg with upstream commonware by SuperFluffy · Pull Request #1759 · tempoxyz/tempo

SuperFluffy · 2025-12-25T23:00:23Z

This patch refactors the DKG actor to bring it in line with the upstream commonware "reshare" example.

This rework was forced by updates commonware to revision 709d24861c4ebe0091e50337f6d2f61d1547fdb1, which is the main branch right after commonwarexyz/monorepo#2439 was merged.

Dealer dealings, player ACKs, and their p2p communications are now no longer defined inside tempo, but employ their upstream versions.

The metadata DB API was removed in favor of journals, as in the commonware example. State management is now capable of dealing with replayed finalized blocks by checking data against an in-memory cache to avoid duplicates and maintain a consistent view. This makes atomic reads/writes unnecessary.

The DKG event loop is now also implemented in terms of a nested loop:

The outer loop iterates over epochs, while the inner loop iterates over finalized blocks.

Closes #1527
Closes #1566
Closes #1650
Closes #1672

vercel · 2025-12-25T23:00:28Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
tempo-docs	Ignored	Preview	Dec 29, 2025 7:14pm

This patch refactors the DKG actor to bring it in line with the upstream commonware "reshare" example. This rework was forced by updates commonware to revision 709d24861c4ebe0091e50337f6d2f61d1547fdb1, which is the main branch right after commonwarexyz/monorepo#2439 was merged. Dealer dealings, player ACKs, and their p2p communications are now no longer defined inside tempo, but employ their upstream versions. The metadata DB API was removed in favor of journals, as in the commonware example. State management is now capable of dealing with replayed finalized blocks by checking data against an in-memory cache to avoid duplicates and maintain a consistent view. This makes atomic reads/writes unnecessary. The DKG event loop is now also implemented in terms of a nested loop: The outer loop iterates over epochs, while the inner loop iterates over finalized blocks.

joshieDo · 2025-12-29T15:52:58Z

+pub struct OnchainDkgOutcome {
+    pub epoch: Epoch,
+    pub output: Output<MinSig, PublicKey>,
+    pub next_players: ordered::Set<PublicKey>,


maybe on write/read we could just set next_players to some kind of diff against the output.players -p1,+p2 etc

That seems like a reasonable optimization - but let's do that once we have a significant number of validators so that we need to start shaving off bytes?

joshieDo · 2025-12-29T16:16:08Z

+        let all_peers = state.construct_merged_peer_set();
+        self.metrics.peers.set(all_peers.len() as i64);
+        self.config
+            .peer_manager
+            .update(state.epoch.get(), all_peers)
+            .await;


unsure on the implications right now but we're including inactive peers here as well, if a dealer/player was set to inactive in the contract, maybe we shouldnt connect to it ?

So the "syncers" are actually set by filtering all validators read from the contract. Even though peers are toggled off on-chain, they might still be participants for the purpose of consensus. For example, if the resharing process failed, we fall back to the dealer.

joshieDo · 2025-12-29T16:36:41Z

-        parent: (View, Digest),
-        round: Round,
-    ) -> eyre::Result<PublicOutcome> {
+        digest: Digest,


super nit: can we call these things block_hash or parent_hash instead of digest? i was lost on what this digest referred to when first glancing at it

I feel you. I am following commonware nomenclature, and they are using digest instead of hash. Right now, we are referring to hash when it comes to talking to the EL, and use digest at the CL level.

If I do the change, I wonder if it would be equally confusing that I talk about hash but sometimes it's a B256 and sometimes wrapped in a Digest. :-/

joshieDo · 2025-12-29T16:39:43Z

        ensure!(
-            dkg_manager
-                .verify_intermediate_dealings(dealing)
-                .await
-                .wrap_err("failed request to verify DKG dealing")?,
-            "signature of intermediate DKG outcome could not be verified",
+            &dealer == proposer,
+            "proposer `{proposer}` is not the dealer `{dealer}` of the dealing \
+            in the block",
        );


while odd, is it a big deal?

Well, I think we shouldn't permit a validator that is not the proposer to include dealer logs. We originally introduced this because the old commonware DKG logic would panic if you finalized it against dealings of unknown dealers. So this patch surfaced this check here, but it's technically not new logic.

I am not sure how this is still an attack vector. I think when checking the log against the current round the log would fail (well, unless the proposer acquired another dealer's signing key...).

joshieDo · 2025-12-29T17:41:58Z

+        me: PrivateKey,
+        round: Round,
+        share: Option<Share>,
+        seed: Summary,


summary instead of seed ? (also for State { .. seed: })

it can be a tad confusing either way, so id say lets stick to the types name

seed makes sense in that it's named after how it's used: state.seed is used an RNG source. So it kinda does make sense?

…iled

read_from_contract_on_epoch_boundary issue

Updates commonware to v0.0.64. There are no functional changes, but a plethora of new types and refactors due to changing Commonware APIs. Highlights: 1. The static `epoch_length` was replaced with a `FixedEpocher`, which is now passed to all our actors. All epoch utilities were also removed. 2. Rate limiters are now all operating at the p2p channel level and so were removed from application logic (we only used them in the epoch manager actor) 3. `application::Mailbox` also needs to implement `CertifiableAutomaton` (no other code changes though) 4. public polynomials are now called `Sharing`. 5. a few traits are now imported from the new `commonware_math` crate. 6. we can now make use of `dkg::deal` and `dkg::deal_anonymous`, simplifying test code and xtask NOTE: this is marked as breaking because in between PR #1759 and this patch commonware had a few other changes going into their 0.0.64 release. For example, commonwarexyz/monorepo#2555

…1905) Removes `--consensus.delete-signing-share`. It was no longer used since #1759, so this is not breaking in any way.

This patch refactors the DKG actor to bring it in line with the upstream commonware "reshare" example. This rework was forced by updates commonware to revision 709d24861c4ebe0091e50337f6d2f61d1547fdb1, which is the main branch right after commonwarexyz/monorepo#2439 was merged. Dealer dealings, player ACKs, and their p2p communications are now no longer defined inside tempo, but employ their upstream versions. The metadata DB API was removed in favor of journals, as in the commonware example. State management is now capable of dealing with replayed finalized blocks by checking data against an in-memory cache to avoid duplicates and maintain a consistent view. This makes atomic reads/writes unnecessary. The DKG event loop is now also implemented in terms of a nested loop: The outer loop iterates over epochs, while the inner loop iterates over finalized blocks. Closes #1527 Closes #1566 Closes #1650 Closes #1672

Updates commonware to v0.0.64. There are no functional changes, but a plethora of new types and refactors due to changing Commonware APIs. Highlights: 1. The static `epoch_length` was replaced with a `FixedEpocher`, which is now passed to all our actors. All epoch utilities were also removed. 2. Rate limiters are now all operating at the p2p channel level and so were removed from application logic (we only used them in the epoch manager actor) 3. `application::Mailbox` also needs to implement `CertifiableAutomaton` (no other code changes though) 4. public polynomials are now called `Sharing`. 5. a few traits are now imported from the new `commonware_math` crate. 6. we can now make use of `dkg::deal` and `dkg::deal_anonymous`, simplifying test code and xtask NOTE: this is marked as breaking because in between PR #1759 and this patch commonware had a few other changes going into their 0.0.64 release. For example, commonwarexyz/monorepo#2555

…1905) Removes `--consensus.delete-signing-share`. It was no longer used since #1759, so this is not breaking in any way.

mergify bot added the S-breaking-stf This PR includes a breaking STF change label Dec 25, 2025

SuperFluffy force-pushed the janis/cw-709d24861c4ebe0091e50337f6d2f61d1547fdb1 branch 3 times, most recently from 8b95fe2 to 15ea1d4 Compare December 25, 2025 23:29

SuperFluffy marked this pull request as ready for review December 25, 2025 23:29

SuperFluffy requested review from 0xKitsune, Zygimantass and joshieDo as code owners December 25, 2025 23:29

SuperFluffy force-pushed the janis/cw-709d24861c4ebe0091e50337f6d2f61d1547fdb1 branch from 15ea1d4 to fa050f2 Compare December 25, 2025 23:30

Base automatically changed from janis/cw-4a5fca0223772b1a1627138d970ce231f876e6ff to main December 25, 2025 23:32

SuperFluffy requested review from klkvr and mattsse as code owners December 25, 2025 23:32

SuperFluffy force-pushed the janis/cw-709d24861c4ebe0091e50337f6d2f61d1547fdb1 branch 4 times, most recently from c65de9c to e4b7dca Compare December 29, 2025 11:08

SuperFluffy force-pushed the janis/cw-709d24861c4ebe0091e50337f6d2f61d1547fdb1 branch from e4b7dca to f3138b5 Compare December 29, 2025 11:18