Skip to content

fix(attestation): revert DEFAULT_EXPIRATION_DURATION_SECONDS to 7 days - #3948

Merged
barakeinav1 merged 1 commit into
mainfrom
barak/3947-attestation-expiry-7d
Jul 23, 2026
Merged

fix(attestation): revert DEFAULT_EXPIRATION_DURATION_SECONDS to 7 days#3948
barakeinav1 merged 1 commit into
mainfrom
barak/3947-attestation-expiry-7d

Conversation

@barakeinav1

@barakeinav1 barakeinav1 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

Reverts DEFAULT_EXPIRATION_DURATION_SECONDS (crates/mpc-attestation/src/attestation.rs) from 1 day back to 7 days, and fixes the stale // 1 day inline comment.

Why

The contract stamps each accepted attestation with expiry = block_timestamp + DEFAULT_EXPIRATION_DURATION_SECONDS (TeeState::add_participant). The value was dropped from 7d to 1d in #3626, which leaves almost no margin: if a node can't refresh its attestation for ~24h it gets dropped. This bit us during an Intel PCS incident where the PCK CRL went stale (>7 days without re-sign) and node-side collateral freshness rejected fresh quote generation — with a 1-day expiry, nodes are ~24h from being kicked; the previous 7-day window would have given a week of runway. Nodes re-attest hourly, well within 7 days.

Safety — no node/contract version-skew hazard

Made safe by #3736 (fix(node): confirm submit_participant_info via advancing attestation expiry, already on main). The node used to confirm its submission by reconstructing the attestation's creation time as stored_expiry − DEFAULT_EXPIRATION_DURATION_SECONDS; under skew (contract stamping 7d while a node still assumes 1d) the reconstructed age (~6d) exceeds the 120s freshness bound, so every submission is treated as NotExecuted and retried indefinitely. #3736 replaced that with an expiry-advanced check that doesn't depend on the constant's value, so this 1d→7d change cannot trigger that retry loop. (Holds provided nodes are on the #3736 build.)

Notes

  • Takes effect for new submissions after redeploy; does not retroactively extend attestations already stored on-chain.
  • Node-side confirmation is independent of this constant since fix(node): confirm submit_participant_info via advancing attestation expiry #3736 (it now checks the expiry advanced rather than reconstructing from the value), so contract and node no longer need to agree on it. The only remaining reference in the node is a doc comment.
  • The node-side collateral freshness fix (relaxing MAX_COLLATERAL_AGE / per-field PCK CRL handling) is a separate PR.

Closes #3947

Restores the 7-day attestation validity window that was reduced to 1 day in
 #3626. The contract stamps expiry = block_timestamp + this constant, so a
 1-day window leaves nodes ~24h to refresh before being dropped; 7 days
 restores the intended margin (nodes re-attest hourly, well within it).

Refs #3947
@barakeinav1
barakeinav1 marked this pull request as ready for review July 23, 2026 15:25
Copilot AI review requested due to automatic review settings July 23, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reverts the attestation expiry window in mpc-attestation back to 7 days to restore operational slack during upstream Intel PCS/collateral incidents, and aligns the inline comment with the restored value.

Changes:

  • Restore DEFAULT_EXPIRATION_DURATION_SECONDS from 1 day to 7 days.
  • Update the inline // 1 day comment to // 7 days.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Pull request overview

Reverts DEFAULT_EXPIRATION_DURATION_SECONDS in crates/mpc-attestation/src/attestation.rs from 1 day back to 7 days (and fixes the trailing // 1 day comment). Motivated by an Intel PCS incident where a stale PCK CRL blocked fresh quote generation for longer than 24h; the 7-day window gives nodes a full week of runway while they continue to re-attest hourly.

Changes:

  • Restore DEFAULT_EXPIRATION_DURATION_SECONDS = 60 * 60 * 24 * 7 and align the inline comment.

Reviewed changes

Per-file summary
File Description
crates/mpc-attestation/src/attestation.rs Change constant value from 1 day back to 7 days; update inline comment.

Findings

Verified the safety claim against main:

  • submitted_attestation_landed at crates/node/src/indexer/tx_sender.rs:201-213 confirms Dstack submissions via attestation_expiry_changed (expiry-advanced check) rather than reconstructing creation time from the constant, so a contract stamping 7d while a node still assumes 1d cannot re-trigger the indefinite-retry loop that would have occurred pre-fix(node): confirm submit_participant_info via advancing attestation expiry #3736.
  • AcceptedAttestation::dstack at crates/mpc-attestation/src/attestation.rs:74-75 computes current_timestamp_seconds + DEFAULT_EXPIRATION_DURATION_SECONDS on u64; 7 days is well within safe range.
  • The re-verification tests at crates/mpc-attestation/tests/test_attestation_verification.rs:75,109 and crates/attestation-cli/tests/test_verification.rs:59 reference the constant symbolically, so they scale with the new value.
  • The doc comment at crates/tee-authority/src/tee_authority.rs:239 explicitly notes that MAX_COLLATERAL_AGE is independent of DEFAULT_EXPIRATION_DURATION_SECONDS, so no doc drift here.

No blocking or non-blocking issues found.

✅ Approved

@gilcu3 gilcu3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@barakeinav1
barakeinav1 enabled auto-merge July 23, 2026 16:34
@barakeinav1
barakeinav1 added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit e02c1dd Jul 23, 2026
24 checks passed
@barakeinav1
barakeinav1 deleted the barak/3947-attestation-expiry-7d branch July 23, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(attestation): revert DEFAULT_EXPIRATION_DURATION_SECONDS from 1 day back to 7 days

4 participants