Skip to content

Commit 6a1e1b5

Browse files
committed
docs(node): trim attestation confirmation comments
Tighten the doc comments the review flagged as verbose/distracting.
1 parent a057241 commit 6a1e1b5

2 files changed

Lines changed: 12 additions & 29 deletions

File tree

crates/node/src/indexer/tx_sender.rs

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -181,34 +181,21 @@ async fn submit_tx(
181181
})
182182
}
183183

184-
/// Confirms a `submit_participant_info` landed by checking the stored expiry *changed* from the
185-
/// pre-submit baseline: a successful submit re-stamps the expiry to a new value, while a failed one
186-
/// leaves it untouched. Returns `true` iff `stored_expiry != pre_submit_expiry`, or there is no
187-
/// baseline (`None`, i.e. nothing was stored before).
188-
///
189-
/// We compare for inequality rather than `stored_expiry > baseline`: a contract upgrade that lowers
190-
/// the expiration constant can make a landed submit set an *earlier* expiry than a stale stored
191-
/// entry, which `>` would miss. The only thing that changes our key's expiry other than our own
192-
/// submit is the verifier-rotation cap (#3734) lowering it — a rare race that would read as landed,
193-
/// bounded and self-correcting via the hourly resubmit. Avoids reconstructing the creation time as
194-
/// `expiry - constant`, which breaks under node/contract version skew and under that cap.
195-
// TODO(#1639): confirm via a creation timestamp read from the certificate itself.
184+
/// Whether our `submit_participant_info` landed: a successful submit re-stamps the stored expiry to
185+
/// a new value, a failed one leaves it unchanged, so a change from the baseline (or no prior entry)
186+
/// means it landed. Inequality rather than `>` because a lowered expiry constant can make a landed
187+
/// submit stamp an *earlier* expiry.
188+
// TODO(#1639): match a certificate-derived identity instead of this expiry heuristic.
196189
fn attestation_expiry_changed(pre_submit_expiry: Option<u64>, stored_expiry: u64) -> bool {
197190
match pre_submit_expiry {
198191
Some(expiry_before_submit) => stored_expiry != expiry_before_submit,
199192
None => true,
200193
}
201194
}
202195

203-
/// Whether the attestation we submitted is the one now stored on chain.
204-
///
205-
/// Mock attestations (tests) carry a full identity, so we match `submitted` against `stored`
206-
/// directly. Dstack can't be matched that way: the stored `VerifiedDstackAttestation` is a
207-
/// different type from the submitted `DstackAttestation` and keeps no per-submission identity (no
208-
/// creation time) to compare on — so `submitted` is unused in that arm and we confirm indirectly,
209-
/// via [`attestation_expiry_changed`] against the pre-submit baseline.
210-
// TODO(#1639): give Dstack a real per-submission identity (a certificate creation timestamp) so it
211-
// can be matched directly like Mock, instead of via the expiry-change heuristic.
196+
/// Whether the attestation we submitted is the one now stored. Mock carries a full identity and is
197+
/// matched directly; a Dstack entry has none to match (it's a different stored type), so it's
198+
/// confirmed indirectly via [`attestation_expiry_changed`].
212199
fn submitted_attestation_landed(
213200
pre_submit_expiry: Option<u64>,
214201
stored: &VerifiedAttestation,
@@ -284,8 +271,6 @@ async fn observe_tx_result(
284271
args,
285272
pre_submit_expiry,
286273
} => {
287-
// A successful submit *changes* the stored expiry from the pre-submit baseline
288-
// captured by the caller; confirm by comparing against what is now stored.
289274
let stored_attestation = indexer_state
290275
.view_client
291276
.get_participant_attestation(&indexer_state.mpc_contract_id, &args.tls_public_key)

crates/node/src/tee/remote_attestation.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,8 @@ pub async fn submit_remote_attestation(
4545
tls_public_key,
4646
);
4747

48-
// TODO(#3746): this loop retries the *same* attestation for up to `MAX_RETRY_DURATION` and
49-
// returns an error on timeout (stopping the caller task), so a late success can store a stale
50-
// attestation. #3746 will split this into an inner submit loop and an outer loop that
51-
// re-generates a fresh attestation.
48+
// TODO(#3746): retries the same attestation and errors on timeout, so a late success can store
49+
// a stale one; #3746 splits this into a submit loop and an outer regenerate loop.
5250
let set_attestation = move || {
5351
let tx_sender = tx_sender.clone();
5452
let propose_join_args_clone = submit_participant_info_args.clone();
@@ -194,7 +192,7 @@ pub async fn periodic_attestation_submission<T: TransactionSender + Clone, I: Ti
194192
.read_stored_dstack_expiry(&tls_public_key)
195193
.await
196194
{
197-
Ok(baseline) => baseline, // Some(expiry) = prior attestation; None = none stored yet (e.g. first submit)
195+
Ok(baseline) => baseline, // Some = prior expiry; None = nothing stored yet (first submit) -- both proceed
198196
Err(error) => {
199197
tracing::warn!(%error, "could not read pre-submit attestation baseline; skipping this round");
200198
continue; // next tick. Do NOT submit with an unknown baseline.
@@ -312,7 +310,7 @@ pub async fn monitor_attestation_removal<T: TransactionSender + Clone>(
312310
.read_stored_dstack_expiry(&tls_public_key)
313311
.await
314312
{
315-
Ok(baseline) => baseline, // Some(expiry) = prior attestation; None = none stored yet (e.g. first submit)
313+
Ok(baseline) => baseline, // Some = prior expiry; None = nothing stored yet (first submit) -- both proceed
316314
Err(error) => {
317315
tracing::warn!(%error, "could not read pre-submit attestation baseline; skipping this round");
318316
was_available = is_available;

0 commit comments

Comments
 (0)