[glue/dkg] Serialize Reshare Epoch Artifact Verification - #4459
Conversation
Deploying monorepo with
|
| Latest commit: |
7b3b29b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1262b582.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://precompute-nits.monorepo-eu0.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | 7b3b29b | Aug 14 2026, 08:02 PM |
Benchmark resultsRegressions: ✅ `qmdb::merkleize/v=any::unordered::fixed::mmr k=10000 ch=false s=true cc=true` (2/2 gates passed)
✅ `qmdb::merkleize/v=current::ordered::fixed::mmb chunk=256 k=10000 ch=false s=true cc=true` (2/2 gates passed)
Baseline commit(s): |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9727b18. Configure here.
| } | ||
|
|
||
| // A cache miss transfers the response to the sole active waiter. | ||
| // Sharing the log view tags the waiter and task with the same input. |
There was a problem hiding this comment.
Redundant call-site restatement comments
Low Severity
New comments above pending_logs, the store overlay merge, cache/ceremony reuse, and waiter transfer restate contracts already documented on pending_logs, CachedArtifact/artifact, and ArtifactWork. That adds call-site narration and design justification without new local invariants.
Triggered by learned rule: Storage crate: don't split impl blocks or reorder functions unnecessarily in refactors
Reviewed by Cursor Bugbot for commit 9727b18. Configure here.
patrick-ogrady
changed the title
There was a problem hiding this comment.
SignedDealerLog::check verifies only the self-signature. The pending path therefore admits a valid outsider into pending maps, exact cache keys, and verification inputs. The durable finalized path has the same authorization gap in one-shot DKG because Store::current() is absent, so its conditional membership guard does not reject the outsider before journaling it. Carry the authoritative round dealer set into both paths and reject signers outside that set before applying the first-log rule.
The supplied regression cases both currently fail: the outsider appears in PendingLogs, and Store::has_log reports that it was persisted.
Regression test patch
diff --git a/glue/src/dkg/reshare/actor/inclusion.rs b/glue/src/dkg/reshare/actor/inclusion.rs
index 602fc2afe..59de87907 100644
--- a/glue/src/dkg/reshare/actor/inclusion.rs
+++ b/glue/src/dkg/reshare/actor/inclusion.rs
@@ -1365,15 +1365,19 @@ mod tests {
tests::mocks::{self, MemorySecretStore, TestBlock, TestBlsVariant},
};
use commonware_actor::Feedback;
+ use commonware_codec::{Decode as _, Encode as _, Write as _};
use commonware_consensus::{Reporter, marshal};
use commonware_cryptography::{
Digestible as _, Signer,
bls12381::{
- dkg::feldman_desmedt::{Dealer as CryptoDealer, Verdict},
+ dkg::feldman_desmedt::{
+ Dealer as CryptoDealer, Output, SignedDealerLog, Verdict,
+ },
primitives::sharing::Mode as SharingMode,
},
ed25519::{PrivateKey, PublicKey},
sha256::Sha256,
+ transcript::{Transcript, Version},
};
use commonware_p2p::simulated::{Config as NetworkConfig, Network};
use commonware_parallel::Sequential;
@@ -1470,6 +1474,31 @@ mod tests {
BTreeMap::from([(public_key, log)])
}
+ fn signed_nondealer_log() -> SignedDealerLog<TestBlsVariant, PrivateKey> {
+ let log = dealer_logs(0).into_values().next().expect("dealer log");
+ let participants = players();
+ let summary = Transcript::new(
+ b"_COMMONWARE_CRYPTOGRAPHY_BLS12381_DKG",
+ Version::V0,
+ )
+ .commit(TEST_NAMESPACE)
+ .commit(0u64.encode())
+ .commit(Option::<Output<TestBlsVariant, PublicKey>>::None.encode())
+ .commit(participants.encode())
+ .commit(participants.encode())
+ .summarize();
+ let mut transcript = Transcript::resume(summary, Version::V0).fork(b"log");
+ transcript.commit(log.encode());
+
+ let signer = PrivateKey::from_seed(99);
+ let signature = transcript.sign(&signer);
+ let mut encoded = Vec::new();
+ signer.public_key().write(&mut encoded);
+ log.write(&mut encoded);
+ signature.write(&mut encoded);
+ SignedDealerLog::decode_cfg(encoded.as_slice(), &NZU32!(4)).expect("signed dealer log")
+ }
+
#[test]
fn dropping_verification_aborts_active_task() {
let executor = deterministic::Runner::timed(Duration::from_secs(10));
@@ -1855,6 +1884,80 @@ mod tests {
});
}
+ #[test]
+ fn pending_logs_ignore_valid_signature_from_nondealer() {
+ let executor = deterministic::Runner::default();
+ executor.start(|context| async move {
+ let info = info();
+ let genesis = mocks::genesis_block(signers()[0].public_key());
+ let block_one = TestBlock::new::<Sha256>(
+ genesis.context().clone(),
+ genesis.digest(),
+ Height::new(1),
+ 1,
+ );
+ let block_two = TestBlock::new::<Sha256>(
+ genesis.context().clone(),
+ block_one.digest(),
+ Height::new(2),
+ 2,
+ )
+ .with_payload::<Sha256, _, _>(
+ NZU32!(4),
+ Payload::DealerLog(signed_nondealer_log()),
+ );
+ let parent = block_two.digest();
+ let ancestry = Box::pin(stream::iter([Arc::new(block_two)]));
+ let (mut response_tx, _response_rx) = oneshot::channel::<TestResponse>();
+ let scan = PendingLogScan {
+ epoch: Epoch::zero(),
+ info: &info,
+ epocher: FixedEpocher::new(NZU64!(4)),
+ finalized_tip: Some(FinalizedTip {
+ height: Height::new(1),
+ digest: Some(block_one.digest()),
+ }),
+ final_height: Height::new(3),
+ };
+
+ let logs = pending_logs(scan, parent, ancestry, context.stopped(), &mut response_tx)
+ .await
+ .expect("anchored ancestry");
+ assert!(logs.is_empty(), "nondealer log must not enter pending view");
+ });
+ }
+
+ #[test]
+ fn finalized_nondealer_log_is_not_persisted_without_current_epoch() {
+ let executor = deterministic::Runner::default();
+ executor.start(|context| async move {
+ let epoch = Epoch::zero();
+ let outsider = PrivateKey::from_seed(99).public_key();
+ let mut store = Store::init(
+ context.child("nondealer_store"),
+ "nondealer-store",
+ NZU32!(4),
+ MemorySecretStore::default(),
+ )
+ .await;
+
+ mocks::TestReshareActor::observe_dealer_log(
+ &signers()[0].public_key(),
+ &info(),
+ &mut store,
+ epoch,
+ None,
+ Some(Payload::DealerLog(signed_nondealer_log())),
+ )
+ .await;
+
+ assert!(
+ !store.has_log(epoch, &outsider),
+ "nondealer log must not enter durable DKG view"
+ );
+ });
+ }
+
#[test]
fn pending_logs_stop_at_the_first_unfinalized_block() {There was a problem hiding this comment.
Confirmed, but not changed in this PR. The same admission behavior is present at merge base e0aa3d5: base pending_logs calls log.check then inserts the signer, and base observe_dealer_log has the same Store::current().is_some_and(...) guard; the cryptography, Store, and one-shot DKG owners are unchanged by this candidate. Info::check_dealer_log still rejects the outsider before ceremony selection, so it cannot affect quorum output or the persisted share; the remaining issue is temporary work/cache pollution and bounded one-shot journal pollution. The authoritative owner is Info.dealers, so a complete fix belongs in the Info / SignedDealerLog::check contract or a separate cross-crate ingress change. I kept that pre-existing issue out of this candidate-scoped autofix.
There was a problem hiding this comment.
(overriding tact)
There was a problem hiding this comment.
We made the decision to allows this originally to defer as much as possible until the ceremony check. I thin we can re-evaluate but don't want to do that here.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## cl/precompute-artifacts #4459 +/- ##
===========================================================
+ Coverage 95.48% 95.50% +0.02%
===========================================================
Files 605 605
Lines 273332 273467 +135
Branches 6574 6580 +6
===========================================================
+ Hits 260996 261188 +192
+ Misses 10155 10095 -60
- Partials 2181 2184 +3
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
2 participants
Summary
Invariants
Finalized dealer logs in
Storeare authoritative and are persisted before reporter acknowledgement. Pending ancestry contributes only a temporary view for proposal or verification. The boundary parent remains derived from the typed ancestry head; a materialized request must supply a contiguous chain through every unfinalized inclusion block.Only one verification task is active. Phase-local speculative results are reconstructable after restart and cannot displace a canonical result. Wire and storage formats are unchanged.
Validation
just test -p commonware-gluejust test --profile slow -p commonware-glue reshare_e2e_state_sync_active_player_late_restartjust test --profile slow -p commonware-glue reshare_e2e_late_state_sync_carries_share_across_failurejust clippy -p commonware-gluejust check-docs -p commonware-gluejust check-stability -p commonware-gluejust lintStacked on #4364.