From 3d406fecbc3c54b096f219c7ecb03e73b97708df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Fri, 28 Aug 2026 11:33:52 +0200 Subject: [PATCH 1/8] refactor(node): retry attestation submission only until the next resubmission tick --- crates/node/src/tee/remote_attestation.rs | 94 ++++++-------------- docs/design/attestation-verifier-contract.md | 2 +- 2 files changed, 29 insertions(+), 67 deletions(-) diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index ba68814395..a943d5d028 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -30,9 +30,7 @@ use tokio::sync::watch; const MIN_BACKOFF_DURATION: Duration = Duration::from_millis(100); const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); -const MAX_RETRY_DURATION: Duration = Duration::from_secs(60 * 60 * 12); // 12 hours. const BACKOFF_FACTOR: f32 = 1.5; -const RESUBMISSION_RETRY_DELAY: Duration = Duration::from_secs(60 * 10); // 10 minutes. const ATTESTATION_RESUBMISSION_INTERVAL: Duration = Duration::from_secs(60 * 60); // 1 hour. /// Shared inputs for the attestation-submission background tasks @@ -49,7 +47,8 @@ pub struct AttestationSubmitter { } /// Submits a remote attestation transaction to the MPC contract, retrying with backoff until -/// success or until a fixed retry window elapses, whichever comes first. +/// success or until one resubmission interval elapses, whichever comes first; by then the +/// periodic task submits a freshly generated attestation anyway. /// /// This function repeatedly attempts to submit a [`contract_args::SubmitParticipantInfoArgs`] transaction containing /// the given participant's attestation and TLS public key. It uses the provided @@ -66,8 +65,6 @@ pub async fn submit_remote_attestation( tls_public_key, ); - // TODO(#3746): retries the same attestation and errors on timeout, so a late success can store - // a stale one; #3746 splits this into a submit loop and an outer regenerate loop. let set_attestation = move || { let tx_sender = tx_sender.clone(); let propose_join_args_clone = submit_participant_info_args.clone(); @@ -111,7 +108,7 @@ pub async fn submit_remote_attestation( "failed to submit attestation" ); }) - .timeout(MAX_RETRY_DURATION) + .timeout(ATTESTATION_RESUBMISSION_INTERVAL) .await .context("failed to submit attestation after multiple retry attempts")? } @@ -141,10 +138,9 @@ fn validate_remote_attestation( } impl AttestationSubmitter { - /// Generates a fresh attestation and submits it to the contract, reporting whether it - /// reached the contract so the caller can decide when to try again. Failures are logged, - /// never propagated. - async fn generate_and_submit(&self) -> bool { + /// Generates a fresh attestation and submits it to the contract. Failures are logged, + /// never propagated: the periodic task resubmits within one interval anyway. + async fn generate_and_submit(&self) { let report_data: ReportData = ReportDataV1::new( *self.tls_public_key.as_bytes(), *self.account_public_key.as_bytes(), @@ -158,11 +154,11 @@ impl AttestationSubmitter { Ok(attestation) => attestation, Err(error @ AttestationError::CollateralFetch(_)) => { tracing::warn!(%error, "TEE attestation generation failed"); - return false; + return; } Err(error) => { tracing::error!(%error, "TEE attestation generation failed"); - return false; + return; } }; let allowed_image_hashes: Vec<_> = self @@ -211,9 +207,7 @@ impl AttestationSubmitter { .inc(); if let Err(error) = submission { tracing::error!(?error, "attestation submission failed"); - return false; } - true } } @@ -229,7 +223,9 @@ pub async fn run_periodic_attestation_submission( submitter: AttestationSubmitter, ) { let mut interval = tokio::time::interval(ATTESTATION_RESUBMISSION_INTERVAL); - interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + // Delay keeps generations one interval apart after an overrun; skipping instead would + // re-anchor to the original schedule and could fire the next generation almost immediately. + interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); periodic_attestation_submission(submitter, interval).await } @@ -258,8 +254,8 @@ fn is_node_in_contract_tee_accounts( /// Monitors the contract for TEE attestation removal and triggers resubmission when needed. /// /// This function watches TEE account changes in the contract and resubmits attestations when -/// the node's TEE attestation is no longer available. A failed resubmission is retried after -/// a fixed delay, or sooner on the next TEE-accounts update; this task returns only when the +/// the node's TEE attestation is no longer available. A failed resubmission is not retried +/// here; the periodic task covers it within one interval. This task returns only when the /// watch channel closes. #[tracing::instrument(skip_all)] pub async fn monitor_attestation_removal( @@ -281,18 +277,8 @@ pub async fn monitor_attestation_removal( "starting TEE attestation removal monitoring; initial TEE attestation status" ); - let mut retry_delay = None; - loop { - let changed = match retry_delay.take() { - Some(delay) => tee_accounts_receiver - .changed() - .timeout(delay) - .await - .unwrap_or(Ok(())), - None => tee_accounts_receiver.changed().await, - }; - if changed.is_err() { + if tee_accounts_receiver.changed().await.is_err() { break; } @@ -310,10 +296,7 @@ pub async fn monitor_attestation_removal( %node_account_id, "TEE attestation removed from contract, resubmitting" ); - if !submitter.generate_and_submit().await { - retry_delay = Some(RESUBMISSION_RETRY_DELAY); - continue; - } + submitter.generate_and_submit().await; } was_available = is_available; @@ -567,63 +550,42 @@ mod tests { let handle = setup.spawn_periodic(2); // When - tokio::time::sleep(MAX_RETRY_DURATION + Duration::from_secs(1)).await; + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL + Duration::from_secs(1)).await; setup.sender().set_failing(false); // Then - tokio::time::timeout(MAX_RETRY_DURATION, setup.sender().wait_for_submission()) - .await - .expect("expected a successful submission after the first retry window timed out"); + tokio::time::timeout( + ATTESTATION_RESUBMISSION_INTERVAL, + setup.sender().wait_for_submission(), + ) + .await + .expect("expected a successful submission after the first retry window timed out"); assert_eq!(setup.sender().count(), 1); handle.abort(); } #[tokio::test(start_paused = true)] #[expect(non_snake_case)] - async fn monitor_attestation_removal__should_retry_failed_resubmission_without_further_updates() - { + async fn monitor_attestation_removal__should_not_retry_failed_resubmission() { // Given: the node is removed and the resubmission burns its whole retry window let setup = test_setup(); setup.sender().set_failing(true); let handle = setup.spawn_monitor(); tokio::task::yield_now().await; setup.remove_node_from_tee_accounts(); - tokio::time::sleep(MAX_RETRY_DURATION + Duration::from_secs(1)).await; + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL + Duration::from_secs(1)).await; // When: the submission starts succeeding, with no further TEE-accounts updates setup.sender().set_failing(false); - // Then: the monitor retries on its own delay + // Then: the monitor stays passive; the periodic task owns the retry tokio::time::timeout( - RESUBMISSION_RETRY_DELAY + Duration::from_secs(1), + ATTESTATION_RESUBMISSION_INTERVAL, setup.sender().wait_for_submission(), ) .await - .expect("expected a resubmission after the retry delay with no further updates"); - assert_eq!(setup.sender().count(), 1); - handle.abort(); - } - - #[tokio::test(start_paused = true)] - #[expect(non_snake_case)] - async fn monitor_attestation_removal__should_retry_failed_resubmission_on_next_update() { - // Given: the node is removed and the resubmission burns its whole retry window - let setup = test_setup(); - setup.sender().set_failing(true); - let handle = setup.spawn_monitor(); - tokio::task::yield_now().await; - setup.remove_node_from_tee_accounts(); - tokio::time::sleep(MAX_RETRY_DURATION + Duration::from_secs(1)).await; - - // When: a TEE-accounts update arrives while the node is still absent - setup.sender().set_failing(false); - setup.remove_node_from_tee_accounts(); - - // Then: the update triggers the retry well before the fixed retry delay - tokio::time::timeout(TEST_RESUBMISSION_WAIT, setup.sender().wait_for_submission()) - .await - .expect("expected a resubmission on the update after a failed one"); - assert_eq!(setup.sender().count(), 1); + .expect_err("expected no resubmission without further TEE-accounts updates"); + assert_eq!(setup.sender().count(), 0); handle.abort(); } diff --git a/docs/design/attestation-verifier-contract.md b/docs/design/attestation-verifier-contract.md index 0b78e608ee..5d6c44042c 100644 --- a/docs/design/attestation-verifier-contract.md +++ b/docs/design/attestation-verifier-contract.md @@ -103,7 +103,7 @@ sequenceDiagram #### Caller-side impact -The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence and on attestation-removal events. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at 12 h). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. +The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence and on attestation-removal events. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at the 1-hour resubmission interval). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. #### Handling failures From d66ef6eb8db809dbdc0c6dd61d8a72e8c7b5801a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Mon, 31 Aug 2026 15:57:04 +0200 Subject: [PATCH 2/8] fix(node): retry attestation submission only until the next resubmission tick A submission could be retried for up to 12 hours, so a late success stored a quote that old even though the periodic task had generated fresher ones in the meantime. Cap the retry window at the resubmission interval instead: once it elapses, the next tick supersedes the attempt with a freshly generated attestation. generate_and_submit no longer reports whether it reached the contract; its only consumer was monitor_attestation_removal, removed in #4282. Closes #4280 Closes #3747 --- crates/node/src/tee/remote_attestation.rs | 52 ++++++++++++++------ docs/design/attestation-verifier-contract.md | 5 +- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index 3f54aaf5ee..74f8b77614 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -29,7 +29,6 @@ use tokio::sync::watch; const MIN_BACKOFF_DURATION: Duration = Duration::from_millis(100); const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); -const MAX_RETRY_DURATION: Duration = Duration::from_secs(60 * 60 * 12); // 12 hours. const BACKOFF_FACTOR: f32 = 1.5; const ATTESTATION_RESUBMISSION_INTERVAL: Duration = Duration::from_secs(60 * 60); // 1 hour. @@ -47,7 +46,7 @@ pub struct AttestationSubmitter { } /// Submits a remote attestation transaction to the MPC contract, retrying with backoff until -/// success or until a fixed retry window elapses, whichever comes first. +/// success or until [`ATTESTATION_RESUBMISSION_INTERVAL`] elapses, whichever comes first. /// /// This function repeatedly attempts to submit a [`contract_args::SubmitParticipantInfoArgs`] transaction containing /// the given participant's attestation and TLS public key. It uses the provided @@ -107,7 +106,7 @@ pub async fn submit_remote_attestation( "failed to submit attestation" ); }) - .timeout(MAX_RETRY_DURATION) + .timeout(ATTESTATION_RESUBMISSION_INTERVAL) .await .context("failed to submit attestation after multiple retry attempts")? } @@ -137,10 +136,9 @@ fn validate_remote_attestation( } impl AttestationSubmitter { - /// Generates a fresh attestation and submits it to the contract, reporting whether it - /// reached the contract so the caller can decide when to try again. Failures are logged, - /// never propagated. - async fn generate_and_submit(&self) -> bool { + /// Generates a fresh attestation and submits it to the contract. Failures are logged, never + /// propagated. + async fn generate_and_submit(&self) { let report_data: ReportData = ReportDataV1::new( *self.tls_public_key.as_bytes(), *self.account_public_key.as_bytes(), @@ -154,11 +152,11 @@ impl AttestationSubmitter { Ok(attestation) => attestation, Err(error @ AttestationError::CollateralFetch(_)) => { tracing::warn!(%error, "TEE attestation generation failed"); - return false; + return; } Err(error) => { tracing::error!(%error, "TEE attestation generation failed"); - return false; + return; } }; let allowed_image_hashes: Vec<_> = self @@ -207,9 +205,7 @@ impl AttestationSubmitter { .inc(); if let Err(error) = submission { tracing::error!(?error, "attestation submission failed"); - return false; } - true } } @@ -248,6 +244,7 @@ mod tests { use crate::indexer::tx_sender::{TransactionProcessorError, TransactionStatus}; use crate::tick::MockTicker; use ed25519_dalek::SigningKey; + use mpc_attestation::attestation::MockAttestation; use rand::SeedableRng; use std::sync::{ Arc, Mutex, @@ -406,17 +403,42 @@ mod tests { let handle = setup.spawn_periodic(2); // When - tokio::time::sleep(MAX_RETRY_DURATION + Duration::from_secs(1)).await; + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL + Duration::from_secs(1)).await; setup.sender().set_failing(false); // Then - tokio::time::timeout(MAX_RETRY_DURATION, setup.sender().wait_for_submission()) - .await - .expect("expected a successful submission after the first retry window timed out"); + tokio::time::timeout( + ATTESTATION_RESUBMISSION_INTERVAL, + setup.sender().wait_for_submission(), + ) + .await + .expect("expected a successful submission after the first retry window timed out"); assert_eq!(setup.sender().count(), 1); handle.abort(); } + #[tokio::test(start_paused = true)] + #[expect(non_snake_case)] + async fn submit_remote_attestation__should_stop_retrying_after_one_resubmission_interval() { + // Given + let setup = test_setup(); + setup.sender().set_failing(true); + let started_at = tokio::time::Instant::now(); + + // When + let result = submit_remote_attestation( + setup.sender().clone(), + Attestation::Mock(MockAttestation::Valid), + setup.submitter.tls_public_key.clone(), + None, + ) + .await; + + // Then + result.expect_err("the retry window must elapse"); + assert_eq!(started_at.elapsed(), ATTESTATION_RESUBMISSION_INTERVAL); + } + async fn validate_locally_generated_attestation( config: LocalTeeAuthorityConfig, ) -> Result<(), VerificationError> { diff --git a/docs/design/attestation-verifier-contract.md b/docs/design/attestation-verifier-contract.md index efa2a7ab62..4e81c98348 100644 --- a/docs/design/attestation-verifier-contract.md +++ b/docs/design/attestation-verifier-contract.md @@ -103,7 +103,7 @@ sequenceDiagram #### Caller-side impact -The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence and on attestation-removal events. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at the 1-hour resubmission interval). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. +The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at that same interval). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. #### Handling failures @@ -213,7 +213,7 @@ The expected trigger for a verifier rotation is a discovered bug in the existing Instead, those entries **age out on their own**. Every stored `VerifiedAttestation` carries an `expiry_timestamp_seconds` and [`re_verify`][re-verify] already rejects any entry once it is past — the same check that bounds a legitimate attestation's lifetime also bounds a wrongly-accepted one's. The lever is the expiration duration, today a hardcoded `DEFAULT_EXPIRATION_DURATION_SECONDS` of 7 days, stamped off-chain at attestation time and carried through the DTO into the stored entry (the contract enforces it, it doesn't compute it). This design lowers that constant to a shorter value — 1 day is a reasonable starting point — shipped as a normal contract/node upgrade, not an on-chain votable parameter. A wrongly-accepted entry then rides out for at most ~1 day after the rotation instead of 7, uniformly and with no sweep, flag, or per-entry bookkeeping. -The window must stay well above the honest-node resubmit cadence so nobody is evicted for timing: `mpc-node`'s [`periodic_attestation_submission`][periodic-attestation-submission] resubmits every hour, and [`monitor_attestation_removal`][monitor-attestation-removal] resubmits the moment a node's entry disappears. Honest nodes therefore refresh with comfortable margin while a stale entry nobody refreshes simply lapses. This is deliberately gentler than purging old-verifier entries the instant a rotation lands: an immediate purge down to threshold is effectively expiration zero, where a transient PCCS or connectivity hiccup can knock honest nodes out before they re-attest, threatening liveness. The short window contains the bad entry without that cliff. +The window must stay well above the honest-node resubmit cadence so nobody is evicted for timing: `mpc-node`'s [`periodic_attestation_submission`][periodic-attestation-submission] resubmits every hour. Honest nodes therefore refresh with comfortable margin while a stale entry nobody refreshes simply lapses. This is deliberately gentler than purging old-verifier entries the instant a rotation lands: an immediate purge down to threshold is effectively expiration zero, where a transient PCCS or connectivity hiccup can knock honest nodes out before they re-attest, threatening liveness. The short window contains the bad entry without that cliff. Note what this does *not* do: it bounds future trust in entries the old verifier produced, but does not undo whatever MPC operations a node holding such an entry already participated in. This is containment of future trust, not remediation of past damage, which is out of scope here. @@ -627,7 +627,6 @@ E2E tests in `crates/e2e-tests` deploy the real `tee-verifier` and vote it in du [verify-tee]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1543 [clean-invalid-attestations]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1646 [clean-foreign-chain-data]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1669 -[monitor-attestation-removal]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/node/src/tee/remote_attestation.rs#L209 [submit-participant-info]: https://github.com/near/mpc/blob/efe49230bb66854c55bba080e7610e42f9221506/crates/contract/src/lib.rs#L754-L782 [launcher-pattern]: https://github.com/near/mpc/blob/efe49230bb66854c55bba080e7610e42f9221506/docs/tee-lifecycle.md#upgrade [slack-launcher-discussion]: https://nearone.slack.com/archives/C0B12RKBSAV/p1777897902903889 From 7d22d802a72c0254f1cc9159fa5cdd31311b8e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Mon, 31 Aug 2026 18:00:20 +0200 Subject: [PATCH 3/8] fix(node): end the submission retry at the next tick, not one interval after it starts The retry window was one interval measured from the moment the submission started, so attestation generation time was added on top of it. A slow PCCS collateral fetch pushed the retry past the tick it belonged to, and a late success stored a correspondingly older quote. The ticker now reports when the next round falls due, derived from the interval's own period, and that instant bounds the submission directly. Generation stays outside it, so a generation failure still waits for the next tick. --- crates/node/src/tee/remote_attestation.rs | 34 ++++++++---------- crates/node/src/tick.rs | 43 ++++++++++++++++++++--- 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index 74f8b77614..2d4f7bdaee 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -25,7 +25,7 @@ use tokio_util::time::FutureExt; use mpc_primitives::hash::{LauncherDockerComposeHash, NodeImageHash}; use near_mpc_contract_interface::call_args as contract_args; -use tokio::sync::watch; +use tokio::{sync::watch, time::Instant}; const MIN_BACKOFF_DURATION: Duration = Duration::from_millis(100); const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); @@ -46,17 +46,18 @@ pub struct AttestationSubmitter { } /// Submits a remote attestation transaction to the MPC contract, retrying with backoff until -/// success or until [`ATTESTATION_RESUBMISSION_INTERVAL`] elapses, whichever comes first. +/// success or until the given deadline, whichever comes first. /// /// This function repeatedly attempts to submit a [`contract_args::SubmitParticipantInfoArgs`] transaction containing /// the given participant's attestation and TLS public key. It uses the provided /// [`TransactionSender`] to send the transaction and waits until [`TransactionStatus::Executed`] /// is observed. Returns an error if no attempt succeeds within the retry window. -pub async fn submit_remote_attestation( +async fn submit_remote_attestation( tx_sender: impl TransactionSender, attestation: Attestation, tls_public_key: Ed25519PublicKey, pre_submit_expiry: Option, + deadline: Instant, ) -> anyhow::Result<()> { let submit_participant_info_args = contract_args::SubmitParticipantInfoArgs::new( attestation.into_contract_interface_type(), @@ -106,7 +107,7 @@ pub async fn submit_remote_attestation( "failed to submit attestation" ); }) - .timeout(ATTESTATION_RESUBMISSION_INTERVAL) + .timeout_at(deadline) .await .context("failed to submit attestation after multiple retry attempts")? } @@ -138,7 +139,7 @@ fn validate_remote_attestation( impl AttestationSubmitter { /// Generates a fresh attestation and submits it to the contract. Failures are logged, never /// propagated. - async fn generate_and_submit(&self) { + async fn generate_and_submit(&self, deadline: Instant) { let report_data: ReportData = ReportDataV1::new( *self.tls_public_key.as_bytes(), *self.account_public_key.as_bytes(), @@ -198,6 +199,7 @@ impl AttestationSubmitter { attestation, self.tls_public_key.clone(), pre_submit_expiry, + deadline, ) .await; MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL @@ -233,8 +235,8 @@ async fn periodic_attestation_submission( mut interval_ticker: I, ) { loop { - interval_ticker.tick().await; - submitter.generate_and_submit().await; + let deadline = interval_ticker.tick().await; + submitter.generate_and_submit(deadline).await; } } @@ -244,7 +246,6 @@ mod tests { use crate::indexer::tx_sender::{TransactionProcessorError, TransactionStatus}; use crate::tick::MockTicker; use ed25519_dalek::SigningKey; - use mpc_attestation::attestation::MockAttestation; use rand::SeedableRng; use std::sync::{ Arc, Mutex, @@ -356,7 +357,7 @@ mod tests { fn spawn_periodic(&self, ticks: usize) -> tokio::task::JoinHandle<()> { tokio::spawn(periodic_attestation_submission( self.submitter.clone(), - MockTicker::new(ticks), + MockTicker::new(ticks).with_period(ATTESTATION_RESUBMISSION_INTERVAL), )) } } @@ -419,24 +420,17 @@ mod tests { #[tokio::test(start_paused = true)] #[expect(non_snake_case)] - async fn submit_remote_attestation__should_stop_retrying_after_one_resubmission_interval() { + async fn generate_and_submit__should_stop_retrying_at_the_deadline() { // Given let setup = test_setup(); setup.sender().set_failing(true); - let started_at = tokio::time::Instant::now(); + let deadline = Instant::now() + ATTESTATION_RESUBMISSION_INTERVAL; // When - let result = submit_remote_attestation( - setup.sender().clone(), - Attestation::Mock(MockAttestation::Valid), - setup.submitter.tls_public_key.clone(), - None, - ) - .await; + setup.submitter.generate_and_submit(deadline).await; // Then - result.expect_err("the retry window must elapse"); - assert_eq!(started_at.elapsed(), ATTESTATION_RESUBMISSION_INTERVAL); + assert_eq!(Instant::now(), deadline); } async fn validate_locally_generated_attestation( diff --git a/crates/node/src/tick.rs b/crates/node/src/tick.rs index b3abb3b3c4..4177352c24 100644 --- a/crates/node/src/tick.rs +++ b/crates/node/src/tick.rs @@ -3,16 +3,20 @@ #[cfg(test)] use std::sync::Arc; #[cfg(test)] +use std::time::Duration; +#[cfg(test)] use tokio::sync::Semaphore; +use tokio::time::Instant; -/// Allows repeatedly awaiting for something, like a [`tokio::time::Interval`]. +/// Allows repeatedly awaiting for something, like a [`tokio::time::Interval`]. The returned instant +/// is when the next round falls due, so it bounds the round just started. pub trait Tick { - async fn tick(&mut self); + async fn tick(&mut self) -> Instant; } impl Tick for tokio::time::Interval { - async fn tick(&mut self) { - self.tick().await; + async fn tick(&mut self) -> Instant { + self.tick().await + self.period() } } @@ -21,6 +25,7 @@ impl Tick for tokio::time::Interval { #[derive(Clone)] pub struct MockTicker { scheduled: Arc, + period: Duration, } #[cfg(test)] @@ -28,9 +33,16 @@ impl MockTicker { pub fn new(count: usize) -> Self { Self { scheduled: Arc::new(Semaphore::new(count)), + period: Duration::ZERO, } } + /// Sets how long after each round the next one falls due. + pub fn with_period(mut self, period: Duration) -> Self { + self.period = period; + self + } + /// Lets the loop run `count` more rounds, from its next poll onwards. pub fn schedule(&self, count: usize) { self.scheduled.add_permits(count); @@ -44,7 +56,7 @@ impl MockTicker { #[cfg(test)] impl Tick for MockTicker { - async fn tick(&mut self) { + async fn tick(&mut self) -> Instant { let round = self .scheduled .acquire() @@ -52,5 +64,26 @@ impl Tick for MockTicker { .expect("the Semaphore is never closed"); // Spend the round. round.forget(); + Instant::now() + self.period + } +} + +#[cfg(test)] +#[expect(non_snake_case)] +mod tests { + use super::*; + + #[tokio::test(start_paused = true)] + async fn tick__should_report_the_next_round_as_the_deadline() { + // Given + let period = Duration::from_secs(60); + let mut interval = tokio::time::interval(period); + let started_at = Instant::now(); + + // When + let deadline = Tick::tick(&mut interval).await; + + // Then + assert_eq!(deadline, started_at + period); } } From c8135c0a6609b590fef827495888f05c44b9b65c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Mon, 31 Aug 2026 18:55:58 +0200 Subject: [PATCH 4/8] test(node): cover the generation-failure path of the attestation loop The acceptance criterion "a generation failure just waits for the next hourly tick" was unverified: attestation generation could not be faulted in a test, because TeeAuthority::Local always succeeds and only the Dstack variant does fallible I/O. Inject generation through a GenerateAttestation trait, mirroring the ReadAttestationExpiry seam next to it but generic rather than boxed, so a test double can fail every attempt and count them. The new test drives the loop with a single scheduled round and shows it runs exactly one, submits nothing, and starts the next only once the ticker yields another. --- crates/node/src/tee/remote_attestation.rs | 101 ++++++++++++++++++---- 1 file changed, 85 insertions(+), 16 deletions(-) diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index 2d4f7bdaee..d9b94b57a6 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -1,4 +1,4 @@ -use std::{sync::Arc, time::Duration}; +use std::{future::Future, sync::Arc, time::Duration}; use crate::{ indexer::{ @@ -32,11 +32,27 @@ const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); const BACKOFF_FACTOR: f32 = 1.5; const ATTESTATION_RESUBMISSION_INTERVAL: Duration = Duration::from_secs(60 * 60); // 1 hour. +pub(crate) trait GenerateAttestation: Send + Sync { + fn generate_attestation( + &self, + report_data: ReportData, + ) -> impl Future> + Send; +} + +impl GenerateAttestation for TeeAuthority { + async fn generate_attestation( + &self, + report_data: ReportData, + ) -> Result { + TeeAuthority::generate_attestation(self, report_data).await + } +} + /// Inputs for the attestation-submission background task /// [`run_periodic_attestation_submission`]. #[derive(Clone)] -pub struct AttestationSubmitter { - pub tee_authority: TeeAuthority, +pub struct AttestationSubmitter { + pub tee_authority: A, pub tx_sender: T, pub tls_public_key: Ed25519PublicKey, pub account_public_key: Ed25519PublicKey, @@ -136,7 +152,7 @@ fn validate_remote_attestation( .map(|_| ()) } -impl AttestationSubmitter { +impl AttestationSubmitter { /// Generates a fresh attestation and submits it to the contract. Failures are logged, never /// propagated. async fn generate_and_submit(&self, deadline: Instant) { @@ -219,9 +235,11 @@ fn outcome_label(succeeded: bool) -> &'static str { } } -pub async fn run_periodic_attestation_submission( - submitter: AttestationSubmitter, -) { +pub async fn run_periodic_attestation_submission(submitter: AttestationSubmitter) +where + T: TransactionSender + Clone, + A: GenerateAttestation, +{ let mut interval = tokio::time::interval(ATTESTATION_RESUBMISSION_INTERVAL); interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); periodic_attestation_submission(submitter, interval).await @@ -230,9 +248,9 @@ pub async fn run_periodic_attestation_submission( /// Periodically regenerates and submits this node's attestation. Generation and submission /// failures are logged and retried on the next tick; this task never returns. #[tracing::instrument(skip_all)] -async fn periodic_attestation_submission( - submitter: AttestationSubmitter, - mut interval_ticker: I, +async fn periodic_attestation_submission( + submitter: AttestationSubmitter, + mut interval_ticker: impl Tick, ) { loop { let deadline = interval_ticker.tick().await; @@ -243,18 +261,40 @@ async fn periodic_attestation_submission( #[cfg(test)] mod tests { use super::*; + use crate::async_testing::{MaybeReady, run_future_once}; use crate::indexer::tx_sender::{TransactionProcessorError, TransactionStatus}; use crate::tick::MockTicker; use ed25519_dalek::SigningKey; use rand::SeedableRng; use std::sync::{ Arc, Mutex, - atomic::{AtomicBool, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, }; use tee_authority::tee_authority::{LocalTeeAuthorityConfig, TeeAuthority}; const TEST_SUBMISSION_COUNT: usize = 2; + #[derive(Clone, Default)] + struct FailingAttestationGenerator { + attempts: Arc, + } + + impl FailingAttestationGenerator { + fn attempts(&self) -> usize { + self.attempts.load(Ordering::Relaxed) + } + } + + impl GenerateAttestation for FailingAttestationGenerator { + async fn generate_attestation( + &self, + _report_data: ReportData, + ) -> Result { + self.attempts.fetch_add(1, Ordering::Relaxed); + Err(AttestationError::InvalidEndpoint) + } + } + struct StubAttestationExpiryReader { fail: bool, } @@ -328,17 +368,21 @@ mod tests { ) } - struct TestSetup { - submitter: AttestationSubmitter, + struct TestSetup { + submitter: AttestationSubmitter, } /// Builds an [`AttestationSubmitter`] around a [`MockSender`]. - fn test_setup() -> TestSetup { + fn test_setup() -> TestSetup { + test_setup_with(TeeAuthority::from(LocalTeeAuthorityConfig::default())) + } + + fn test_setup_with(tee_authority: A) -> TestSetup { let (tls_public_key, account_public_key) = test_keys(); let (_, allowed_image_hashes) = watch::channel(vec![]); let (_, allowed_launcher_compose_hashes) = watch::channel(vec![]); let submitter = AttestationSubmitter { - tee_authority: TeeAuthority::from(LocalTeeAuthorityConfig::default()), + tee_authority, tx_sender: MockSender::default(), tls_public_key, account_public_key, @@ -349,7 +393,7 @@ mod tests { TestSetup { submitter } } - impl TestSetup { + impl TestSetup { fn sender(&self) -> &MockSender { &self.submitter.tx_sender } @@ -418,6 +462,31 @@ mod tests { handle.abort(); } + #[test] + #[expect(non_snake_case)] + fn periodic_attestation_submission__should_wait_for_the_next_tick_when_generation_fails() { + // Given + let generator = FailingAttestationGenerator::default(); + let setup = test_setup_with(generator.clone()); + let ticker = MockTicker::new(1); + + // When + let MaybeReady::Future(parked_loop) = run_future_once(periodic_attestation_submission( + setup.submitter.clone(), + ticker.clone(), + )) else { + panic!("the loop should park once its ticker runs out"); + }; + let attempts_before_the_next_tick = generator.attempts(); + ticker.schedule(1); + run_future_once(parked_loop); + + // Then + assert_eq!(attempts_before_the_next_tick, 1); + assert_eq!(generator.attempts(), 2); + assert_eq!(setup.sender().count(), 0); + } + #[tokio::test(start_paused = true)] #[expect(non_snake_case)] async fn generate_and_submit__should_stop_retrying_at_the_deadline() { From df900f730ef77854380e03d636f647133a7b098f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 1 Sep 2026 10:38:02 +0200 Subject: [PATCH 5/8] docs: leave the attestation verifier design doc unchanged Design docs record what was decided at the time and are not kept in sync with the code once the design has shipped, so this branch's edits to the retry window and the attestation-removal references are reverted. --- docs/design/attestation-verifier-contract.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/design/attestation-verifier-contract.md b/docs/design/attestation-verifier-contract.md index 4e81c98348..5c7ea8c222 100644 --- a/docs/design/attestation-verifier-contract.md +++ b/docs/design/attestation-verifier-contract.md @@ -103,7 +103,7 @@ sequenceDiagram #### Caller-side impact -The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at that same interval). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. +The only caller of `submit_participant_info` in production is `mpc-node`'s `periodic_attestation_submission` task, which resubmits on a 1-hour cadence and on attestation-removal events. It already polls contract state to confirm the attestation is actually stored, with exponential backoff (100 ms → 60 s, capped at 12 h). That polling-based success criterion is what makes the sync→async change transparent. Under yield-resume the returned `Promise` also resolves with the actual outcome — success, a verifier-rejection error, or a post-DCAP-failure error (each as soon as the verifier answers), or a timeout error after ~200 blocks if it never does — so any future caller that wants to await the result synchronously can, without changing the contract. #### Handling failures @@ -213,7 +213,7 @@ The expected trigger for a verifier rotation is a discovered bug in the existing Instead, those entries **age out on their own**. Every stored `VerifiedAttestation` carries an `expiry_timestamp_seconds` and [`re_verify`][re-verify] already rejects any entry once it is past — the same check that bounds a legitimate attestation's lifetime also bounds a wrongly-accepted one's. The lever is the expiration duration, today a hardcoded `DEFAULT_EXPIRATION_DURATION_SECONDS` of 7 days, stamped off-chain at attestation time and carried through the DTO into the stored entry (the contract enforces it, it doesn't compute it). This design lowers that constant to a shorter value — 1 day is a reasonable starting point — shipped as a normal contract/node upgrade, not an on-chain votable parameter. A wrongly-accepted entry then rides out for at most ~1 day after the rotation instead of 7, uniformly and with no sweep, flag, or per-entry bookkeeping. -The window must stay well above the honest-node resubmit cadence so nobody is evicted for timing: `mpc-node`'s [`periodic_attestation_submission`][periodic-attestation-submission] resubmits every hour. Honest nodes therefore refresh with comfortable margin while a stale entry nobody refreshes simply lapses. This is deliberately gentler than purging old-verifier entries the instant a rotation lands: an immediate purge down to threshold is effectively expiration zero, where a transient PCCS or connectivity hiccup can knock honest nodes out before they re-attest, threatening liveness. The short window contains the bad entry without that cliff. +The window must stay well above the honest-node resubmit cadence so nobody is evicted for timing: `mpc-node`'s [`periodic_attestation_submission`][periodic-attestation-submission] resubmits every hour, and [`monitor_attestation_removal`][monitor-attestation-removal] resubmits the moment a node's entry disappears. Honest nodes therefore refresh with comfortable margin while a stale entry nobody refreshes simply lapses. This is deliberately gentler than purging old-verifier entries the instant a rotation lands: an immediate purge down to threshold is effectively expiration zero, where a transient PCCS or connectivity hiccup can knock honest nodes out before they re-attest, threatening liveness. The short window contains the bad entry without that cliff. Note what this does *not* do: it bounds future trust in entries the old verifier produced, but does not undo whatever MPC operations a node holding such an entry already participated in. This is containment of future trust, not remediation of past damage, which is out of scope here. @@ -627,6 +627,7 @@ E2E tests in `crates/e2e-tests` deploy the real `tee-verifier` and vote it in du [verify-tee]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1543 [clean-invalid-attestations]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1646 [clean-foreign-chain-data]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/contract/src/lib.rs#L1669 +[monitor-attestation-removal]: https://github.com/near/mpc/blob/5e47bfe93b398cb2343681fa2c0f2691d02c7285/crates/node/src/tee/remote_attestation.rs#L209 [submit-participant-info]: https://github.com/near/mpc/blob/efe49230bb66854c55bba080e7610e42f9221506/crates/contract/src/lib.rs#L754-L782 [launcher-pattern]: https://github.com/near/mpc/blob/efe49230bb66854c55bba080e7610e42f9221506/docs/tee-lifecycle.md#upgrade [slack-launcher-discussion]: https://nearone.slack.com/archives/C0B12RKBSAV/p1777897902903889 From 8c97ae88f483b8c4164de64a0ab972db3e15ff3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 1 Sep 2026 12:29:05 +0200 Subject: [PATCH 6/8] fix(node): bound attestation generation by the round deadline Only the submission was bounded, so a round could run past its deadline in generation and, because tokio reports the instant a tick was scheduled for rather than the current time, a round overrunning by more than one period was handed a deadline already in the past. The submission then gave up without a single retry while still logging that it had exhausted its attempts. Derive the deadline from the tick's own start instead, and apply it to generation and to the pre-submit expiry read as well, so every await in a round shares one absolute deadline. Generation timeouts get their own metric label, since they need a different response than a generation error. The submission keeps bounding itself rather than being wrapped from outside: a timeout there would drop the future, stranding a signed transaction that the sender has already handed to a detached task, and skipping the counter that records the failure. --- crates/node/src/metrics.rs | 1 + crates/node/src/tee/remote_attestation.rs | 78 +++++++++++++++++++---- crates/node/src/tick.rs | 13 ++-- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/crates/node/src/metrics.rs b/crates/node/src/metrics.rs index f8fba99adc..e1e721fc19 100644 --- a/crates/node/src/metrics.rs +++ b/crates/node/src/metrics.rs @@ -489,6 +489,7 @@ pub static MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL: LazyLock = LazyLock::new(|| { diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index d9b94b57a6..e685dd1d3c 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -7,8 +7,9 @@ use crate::{ types::ChainSendTransactionRequest, }, metrics::{ - MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL, MPC_TEE_ATTESTATION_OUTCOME_FAILURE, - MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL, + MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL, MPC_TEE_ATTESTATION_OUTCOME_DEADLINE_EXCEEDED, + MPC_TEE_ATTESTATION_OUTCOME_FAILURE, MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, + MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL, }, tick::Tick, trait_extensions::convert_to_contract_dto::IntoContractInterfaceType, @@ -161,20 +162,33 @@ impl AttestationSubmitter< *self.account_public_key.as_bytes(), ) .into(); - let result = self.tee_authority.generate_attestation(report_data).await; + let generated = self + .tee_authority + .generate_attestation(report_data) + .timeout_at(deadline) + .await; + let generation_outcome = match &generated { + Ok(Ok(_)) => MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, + Ok(Err(_)) => MPC_TEE_ATTESTATION_OUTCOME_FAILURE, + Err(_) => MPC_TEE_ATTESTATION_OUTCOME_DEADLINE_EXCEEDED, + }; MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL - .with_label_values(&[outcome_label(result.is_ok())]) + .with_label_values(&[generation_outcome]) .inc(); - let attestation = match result { - Ok(attestation) => attestation, - Err(error @ AttestationError::CollateralFetch(_)) => { - tracing::warn!(%error, "TEE attestation generation failed"); + let attestation = match generated { + Ok(Ok(attestation)) => attestation, + Ok(Err(error @ AttestationError::CollateralFetch(_))) => { + tracing::warn!(%error, "TEE attestation collateral fetch failed"); return; } - Err(error) => { + Ok(Err(error)) => { tracing::error!(%error, "TEE attestation generation failed"); return; } + Err(error) => { + tracing::error!(%error, "TEE attestation generation timed out"); + return; + } }; let allowed_image_hashes: Vec<_> = self .allowed_image_hashes @@ -186,7 +200,9 @@ impl AttestationSubmitter< let pre_submit_expiry = match self .attestation_reader .read_stored_attestation_expiry(&self.tls_public_key) + .timeout_at(deadline) .await + .unwrap_or_else(|elapsed| Err(elapsed.into())) { Ok(baseline) => baseline, // None just means nothing stored yet (e.g. first submit) // Submit anyway on a read error: refreshing the attestation is the priority, and a @@ -295,6 +311,18 @@ mod tests { } } + #[derive(Clone)] + struct HangingAttestationGenerator; + + impl GenerateAttestation for HangingAttestationGenerator { + async fn generate_attestation( + &self, + _report_data: ReportData, + ) -> Result { + std::future::pending().await + } + } + struct StubAttestationExpiryReader { fail: bool, } @@ -318,6 +346,7 @@ mod tests { #[derive(Clone, Default)] struct MockSender { + attempts: Arc, submissions: Arc>, notify: Arc, failing: Arc, @@ -328,6 +357,10 @@ mod tests { self.failing.store(failing, Ordering::Relaxed); } + fn attempts(&self) -> usize { + self.attempts.load(Ordering::Relaxed) + } + fn count(&self) -> usize { *self.submissions.lock().unwrap() } @@ -342,6 +375,7 @@ mod tests { &self, _: ChainSendTransactionRequest, ) -> Result<(), TransactionProcessorError> { + self.attempts.fetch_add(1, Ordering::Relaxed); if self.failing.load(Ordering::Relaxed) { return Err(TransactionProcessorError::ProcessorIsClosed); } @@ -462,9 +496,10 @@ mod tests { handle.abort(); } - #[test] + #[tokio::test] #[expect(non_snake_case)] - fn periodic_attestation_submission__should_wait_for_the_next_tick_when_generation_fails() { + async fn periodic_attestation_submission__should_wait_for_the_next_tick_when_generation_fails() + { // Given let generator = FailingAttestationGenerator::default(); let setup = test_setup_with(generator.clone()); @@ -500,6 +535,27 @@ mod tests { // Then assert_eq!(Instant::now(), deadline); + assert!(setup.sender().attempts() > 1); + } + + #[tokio::test(start_paused = true)] + #[expect(non_snake_case)] + async fn generate_and_submit__should_stop_generating_at_the_deadline() { + // Given + let setup = test_setup_with(HangingAttestationGenerator); + let deadline = Instant::now() + ATTESTATION_RESUBMISSION_INTERVAL; + + // When + tokio::time::timeout( + 2 * ATTESTATION_RESUBMISSION_INTERVAL, + setup.submitter.generate_and_submit(deadline), + ) + .await + .expect("generation should have stopped at the deadline"); + + // Then + assert_eq!(Instant::now(), deadline); + assert_eq!(setup.sender().count(), 0); } async fn validate_locally_generated_attestation( diff --git a/crates/node/src/tick.rs b/crates/node/src/tick.rs index 4177352c24..8d0e175070 100644 --- a/crates/node/src/tick.rs +++ b/crates/node/src/tick.rs @@ -9,14 +9,15 @@ use tokio::sync::Semaphore; use tokio::time::Instant; /// Allows repeatedly awaiting for something, like a [`tokio::time::Interval`]. The returned instant -/// is when the next round falls due, so it bounds the round just started. +/// is one period after the round it starts. pub trait Tick { async fn tick(&mut self) -> Instant; } impl Tick for tokio::time::Interval { async fn tick(&mut self) -> Instant { - self.tick().await + self.period() + self.tick().await; + Instant::now() + self.period() } } @@ -74,16 +75,18 @@ mod tests { use super::*; #[tokio::test(start_paused = true)] - async fn tick__should_report_the_next_round_as_the_deadline() { + async fn tick__should_bound_the_round_from_its_start_after_a_missed_tick() { // Given let period = Duration::from_secs(60); let mut interval = tokio::time::interval(period); - let started_at = Instant::now(); + interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + Tick::tick(&mut interval).await; + tokio::time::sleep(2 * period + Duration::from_secs(1)).await; // When let deadline = Tick::tick(&mut interval).await; // Then - assert_eq!(deadline, started_at + period); + assert_eq!(deadline, Instant::now() + period); } } From f3d138b3ba3c5e5dcc4891f908f042e65995670d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 1 Sep 2026 17:12:43 +0200 Subject: [PATCH 7/8] refactor(node): bound the attestation round from the loop instead of threading a deadline The loop now cancels a round with one timeout instead of passing a deadline into generate_and_submit, and the Tick trait returns to being a pure tick. The round publishes the stage it is in through a watch channel, so a timed-out round is logged as a warning and counted in mpc_tee_attestation_round_timeouts_total under the stage that was running when the round was cut off. --- crates/node/src/metrics.rs | 15 +- crates/node/src/tee/remote_attestation.rs | 173 ++++++++++++++-------- crates/node/src/tick.rs | 44 +----- 3 files changed, 129 insertions(+), 103 deletions(-) diff --git a/crates/node/src/metrics.rs b/crates/node/src/metrics.rs index e1e721fc19..eb41612dcc 100644 --- a/crates/node/src/metrics.rs +++ b/crates/node/src/metrics.rs @@ -489,7 +489,20 @@ pub static MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL: LazyLock = + LazyLock::new(|| { + prometheus::register_int_counter_vec!( + "mpc_tee_attestation_round_timeouts_total", + "Total number of TEE attestation submission rounds that timed out, by stage", + &["stage"], + ) + .unwrap() + }); + +pub const MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION: &str = "generate_attestation"; +pub const MPC_TEE_ATTESTATION_STAGE_READ_EXPIRY_BASELINE: &str = "read_expiry_baseline"; +pub const MPC_TEE_ATTESTATION_STAGE_SUBMIT_ATTESTATION: &str = "submit_attestation"; pub static FOREIGN_CHAIN_RPC_PROVIDERS_CONFIGURED: LazyLock = LazyLock::new(|| { diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index e685dd1d3c..05e202a88f 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -7,9 +7,11 @@ use crate::{ types::ChainSendTransactionRequest, }, metrics::{ - MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL, MPC_TEE_ATTESTATION_OUTCOME_DEADLINE_EXCEEDED, - MPC_TEE_ATTESTATION_OUTCOME_FAILURE, MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, - MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL, + MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL, MPC_TEE_ATTESTATION_OUTCOME_FAILURE, + MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL, + MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION, + MPC_TEE_ATTESTATION_STAGE_READ_EXPIRY_BASELINE, + MPC_TEE_ATTESTATION_STAGE_SUBMIT_ATTESTATION, MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL, }, tick::Tick, trait_extensions::convert_to_contract_dto::IntoContractInterfaceType, @@ -22,11 +24,10 @@ use mpc_attestation::{ }; use near_mpc_contract_interface::types::{AllowedMpcDockerImageHash, Ed25519PublicKey}; use tee_authority::tee_authority::{AttestationError, TeeAuthority}; -use tokio_util::time::FutureExt; use mpc_primitives::hash::{LauncherDockerComposeHash, NodeImageHash}; use near_mpc_contract_interface::call_args as contract_args; -use tokio::{sync::watch, time::Instant}; +use tokio::sync::watch; const MIN_BACKOFF_DURATION: Duration = Duration::from_millis(100); const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); @@ -62,19 +63,36 @@ pub struct AttestationSubmitter { pub attestation_reader: Arc, } -/// Submits a remote attestation transaction to the MPC contract, retrying with backoff until -/// success or until the given deadline, whichever comes first. -/// -/// This function repeatedly attempts to submit a [`contract_args::SubmitParticipantInfoArgs`] transaction containing -/// the given participant's attestation and TLS public key. It uses the provided -/// [`TransactionSender`] to send the transaction and waits until [`TransactionStatus::Executed`] -/// is observed. Returns an error if no attempt succeeds within the retry window. +/// The stage a single iteration of [`periodic_attestation_submission`] is in. A cancelled +/// iteration cannot report anything itself, so it publishes each stage it enters, and the one +/// it was cancelled in ends up in the log and metric. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum RoundStage { + GenerateAttestation, + ReadExpiryBaseline, + SubmitAttestation, +} + +impl RoundStage { + fn as_label(self) -> &'static str { + match self { + RoundStage::GenerateAttestation => MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION, + RoundStage::ReadExpiryBaseline => MPC_TEE_ATTESTATION_STAGE_READ_EXPIRY_BASELINE, + RoundStage::SubmitAttestation => MPC_TEE_ATTESTATION_STAGE_SUBMIT_ATTESTATION, + } + } +} + +/// Submits a [`contract_args::SubmitParticipantInfoArgs`] transaction containing the given +/// participant's attestation and TLS public key, using the provided [`TransactionSender`] and +/// retrying with backoff until [`TransactionStatus::Executed`] is observed. Retries are +/// unbounded; the caller stops them by dropping this future, as +/// [`periodic_attestation_submission`] does when a round times out. async fn submit_remote_attestation( tx_sender: impl TransactionSender, attestation: Attestation, tls_public_key: Ed25519PublicKey, pre_submit_expiry: Option, - deadline: Instant, ) -> anyhow::Result<()> { let submit_participant_info_args = contract_args::SubmitParticipantInfoArgs::new( attestation.into_contract_interface_type(), @@ -124,9 +142,7 @@ async fn submit_remote_attestation( "failed to submit attestation" ); }) - .timeout_at(deadline) .await - .context("failed to submit attestation after multiple retry attempts")? } fn validate_remote_attestation( @@ -154,39 +170,28 @@ fn validate_remote_attestation( } impl AttestationSubmitter { - /// Generates a fresh attestation and submits it to the contract. Failures are logged, never - /// propagated. - async fn generate_and_submit(&self, deadline: Instant) { + /// Generates a fresh attestation and submits it to the contract, publishing each stage it + /// enters to the given [`watch::Sender`] so a caller that drops this future can tell + /// which stage was cut off. Failures are logged, never propagated. + async fn generate_and_submit(&self, stage: &watch::Sender) { let report_data: ReportData = ReportDataV1::new( *self.tls_public_key.as_bytes(), *self.account_public_key.as_bytes(), ) .into(); - let generated = self - .tee_authority - .generate_attestation(report_data) - .timeout_at(deadline) - .await; - let generation_outcome = match &generated { - Ok(Ok(_)) => MPC_TEE_ATTESTATION_OUTCOME_SUCCESS, - Ok(Err(_)) => MPC_TEE_ATTESTATION_OUTCOME_FAILURE, - Err(_) => MPC_TEE_ATTESTATION_OUTCOME_DEADLINE_EXCEEDED, - }; + stage.send_replace(RoundStage::GenerateAttestation); + let result = self.tee_authority.generate_attestation(report_data).await; MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL - .with_label_values(&[generation_outcome]) + .with_label_values(&[outcome_label(result.is_ok())]) .inc(); - let attestation = match generated { - Ok(Ok(attestation)) => attestation, - Ok(Err(error @ AttestationError::CollateralFetch(_))) => { + let attestation = match result { + Ok(attestation) => attestation, + Err(error @ AttestationError::CollateralFetch(_)) => { tracing::warn!(%error, "TEE attestation collateral fetch failed"); return; } - Ok(Err(error)) => { - tracing::error!(%error, "TEE attestation generation failed"); - return; - } Err(error) => { - tracing::error!(%error, "TEE attestation generation timed out"); + tracing::error!(%error, "TEE attestation generation failed"); return; } }; @@ -197,12 +202,11 @@ impl AttestationSubmitter< .map(|entry| entry.image_hash) .collect(); let allowed_launcher_compose_hashes = self.allowed_launcher_compose_hashes.borrow().clone(); + stage.send_replace(RoundStage::ReadExpiryBaseline); let pre_submit_expiry = match self .attestation_reader .read_stored_attestation_expiry(&self.tls_public_key) - .timeout_at(deadline) .await - .unwrap_or_else(|elapsed| Err(elapsed.into())) { Ok(baseline) => baseline, // None just means nothing stored yet (e.g. first submit) // Submit anyway on a read error: refreshing the attestation is the priority, and a @@ -226,12 +230,12 @@ impl AttestationSubmitter< // fail on a stale view of the allowed hashes tracing::warn!("Attestation is not valid: {error}"); } + stage.send_replace(RoundStage::SubmitAttestation); let submission = submit_remote_attestation( self.tx_sender.clone(), attestation, self.tls_public_key.clone(), pre_submit_expiry, - deadline, ) .await; MPC_TEE_ATTESTATION_SUBMISSIONS_TOTAL @@ -258,19 +262,34 @@ where { let mut interval = tokio::time::interval(ATTESTATION_RESUBMISSION_INTERVAL); interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - periodic_attestation_submission(submitter, interval).await + periodic_attestation_submission(submitter, interval, ATTESTATION_RESUBMISSION_INTERVAL).await } -/// Periodically regenerates and submits this node's attestation. Generation and submission -/// failures are logged and retried on the next tick; this task never returns. +/// Periodically regenerates and submits this node's attestation. A round that times out is +/// abandoned, recorded in [`MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL`] under the stage it was +/// cut in, and the next tick starts over with a fresh attestation. Failures are logged, +/// never propagated; this task never returns. #[tracing::instrument(skip_all)] async fn periodic_attestation_submission( submitter: AttestationSubmitter, mut interval_ticker: impl Tick, + round_timeout: Duration, ) { loop { - let deadline = interval_ticker.tick().await; - submitter.generate_and_submit(deadline).await; + interval_ticker.tick().await; + let (stage, _) = watch::channel(RoundStage::GenerateAttestation); + let round = tokio::time::timeout(round_timeout, submitter.generate_and_submit(&stage)); + if round.await.is_err() { + let stage = stage.borrow().as_label(); + MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL + .with_label_values(&[stage]) + .inc(); + tracing::warn!( + stage, + timeout = ?round_timeout, + "attestation round timed out; retrying from scratch on the next tick" + ); + } } } @@ -435,7 +454,8 @@ mod tests { fn spawn_periodic(&self, ticks: usize) -> tokio::task::JoinHandle<()> { tokio::spawn(periodic_attestation_submission( self.submitter.clone(), - MockTicker::new(ticks).with_period(ATTESTATION_RESUBMISSION_INTERVAL), + MockTicker::new(ticks), + ATTESTATION_RESUBMISSION_INTERVAL, )) } } @@ -476,7 +496,7 @@ mod tests { #[tokio::test(start_paused = true)] #[expect(non_snake_case)] async fn periodic_attestation_submission__should_survive_submission_retry_timeout() { - // Given: the first tick burns the whole retry window, the second one lands + // Given: the first round times out, the second one lands let setup = test_setup(); setup.sender().set_failing(true); let handle = setup.spawn_periodic(2); @@ -491,7 +511,7 @@ mod tests { setup.sender().wait_for_submission(), ) .await - .expect("expected a successful submission after the first retry window timed out"); + .expect("expected a successful submission on the round after the cut-off one"); assert_eq!(setup.sender().count(), 1); handle.abort(); } @@ -509,6 +529,7 @@ mod tests { let MaybeReady::Future(parked_loop) = run_future_once(periodic_attestation_submission( setup.submitter.clone(), ticker.clone(), + ATTESTATION_RESUBMISSION_INTERVAL, )) else { panic!("the loop should park once its ticker runs out"); }; @@ -524,38 +545,66 @@ mod tests { #[tokio::test(start_paused = true)] #[expect(non_snake_case)] - async fn generate_and_submit__should_stop_retrying_at_the_deadline() { + async fn periodic_attestation_submission__should_stop_submission_retries_at_the_round_timeout() + { // Given let setup = test_setup(); setup.sender().set_failing(true); - let deadline = Instant::now() + ATTESTATION_RESUBMISSION_INTERVAL; + let handle = setup.spawn_periodic(1); // When - setup.submitter.generate_and_submit(deadline).await; + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL + Duration::from_secs(1)).await; + let attempts_at_the_timeout = setup.sender().attempts(); + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL).await; // Then - assert_eq!(Instant::now(), deadline); - assert!(setup.sender().attempts() > 1); + assert!(attempts_at_the_timeout > 1); + assert_eq!(setup.sender().attempts(), attempts_at_the_timeout); + handle.abort(); } #[tokio::test(start_paused = true)] #[expect(non_snake_case)] - async fn generate_and_submit__should_stop_generating_at_the_deadline() { + async fn periodic_attestation_submission__should_cut_hung_generation_at_the_round_timeout() { // Given let setup = test_setup_with(HangingAttestationGenerator); - let deadline = Instant::now() + ATTESTATION_RESUBMISSION_INTERVAL; + let cut_rounds_before = MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL + .with_label_values(&[MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION]) + .get(); + let handle = setup.spawn_periodic(1); // When - tokio::time::timeout( - 2 * ATTESTATION_RESUBMISSION_INTERVAL, - setup.submitter.generate_and_submit(deadline), - ) - .await - .expect("generation should have stopped at the deadline"); + tokio::time::sleep(ATTESTATION_RESUBMISSION_INTERVAL + Duration::from_secs(1)).await; // Then - assert_eq!(Instant::now(), deadline); assert_eq!(setup.sender().count(), 0); + assert_eq!( + MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL + .with_label_values(&[MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION]) + .get(), + cut_rounds_before + 1 + ); + handle.abort(); + } + + #[tokio::test(start_paused = true)] + #[expect(non_snake_case)] + async fn generate_and_submit__should_record_the_submission_stage_when_cut_off() { + // Given + let setup = test_setup(); + setup.sender().set_failing(true); + let (stage, _) = watch::channel(RoundStage::GenerateAttestation); + + // When + let round = tokio::time::timeout( + ATTESTATION_RESUBMISSION_INTERVAL, + setup.submitter.generate_and_submit(&stage), + ) + .await; + + // Then + assert!(round.is_err()); + assert_eq!(*stage.borrow(), RoundStage::SubmitAttestation); } async fn validate_locally_generated_attestation( diff --git a/crates/node/src/tick.rs b/crates/node/src/tick.rs index 8d0e175070..b3abb3b3c4 100644 --- a/crates/node/src/tick.rs +++ b/crates/node/src/tick.rs @@ -3,21 +3,16 @@ #[cfg(test)] use std::sync::Arc; #[cfg(test)] -use std::time::Duration; -#[cfg(test)] use tokio::sync::Semaphore; -use tokio::time::Instant; -/// Allows repeatedly awaiting for something, like a [`tokio::time::Interval`]. The returned instant -/// is one period after the round it starts. +/// Allows repeatedly awaiting for something, like a [`tokio::time::Interval`]. pub trait Tick { - async fn tick(&mut self) -> Instant; + async fn tick(&mut self); } impl Tick for tokio::time::Interval { - async fn tick(&mut self) -> Instant { + async fn tick(&mut self) { self.tick().await; - Instant::now() + self.period() } } @@ -26,7 +21,6 @@ impl Tick for tokio::time::Interval { #[derive(Clone)] pub struct MockTicker { scheduled: Arc, - period: Duration, } #[cfg(test)] @@ -34,16 +28,9 @@ impl MockTicker { pub fn new(count: usize) -> Self { Self { scheduled: Arc::new(Semaphore::new(count)), - period: Duration::ZERO, } } - /// Sets how long after each round the next one falls due. - pub fn with_period(mut self, period: Duration) -> Self { - self.period = period; - self - } - /// Lets the loop run `count` more rounds, from its next poll onwards. pub fn schedule(&self, count: usize) { self.scheduled.add_permits(count); @@ -57,7 +44,7 @@ impl MockTicker { #[cfg(test)] impl Tick for MockTicker { - async fn tick(&mut self) -> Instant { + async fn tick(&mut self) { let round = self .scheduled .acquire() @@ -65,28 +52,5 @@ impl Tick for MockTicker { .expect("the Semaphore is never closed"); // Spend the round. round.forget(); - Instant::now() + self.period - } -} - -#[cfg(test)] -#[expect(non_snake_case)] -mod tests { - use super::*; - - #[tokio::test(start_paused = true)] - async fn tick__should_bound_the_round_from_its_start_after_a_missed_tick() { - // Given - let period = Duration::from_secs(60); - let mut interval = tokio::time::interval(period); - interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - Tick::tick(&mut interval).await; - tokio::time::sleep(2 * period + Duration::from_secs(1)).await; - - // When - let deadline = Tick::tick(&mut interval).await; - - // Then - assert_eq!(deadline, Instant::now() + period); } } From 086dae059baf81ab197e74a99cdb9cd87138ce26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Wed, 2 Sep 2026 10:16:45 +0200 Subject: [PATCH 8/8] refactor(node): unroll the attestation round into per-stage timeouts in the loop generate_and_submit is split into the three stage methods and the loop drives them directly, each bounded by timeout_at against one loop-local deadline. The stage enum, the watch channel, and the round timeout parameter are gone; each timeout site names its own stage label. --- crates/node/src/tee/remote_attestation.rs | 136 +++++++++------------- 1 file changed, 54 insertions(+), 82 deletions(-) diff --git a/crates/node/src/tee/remote_attestation.rs b/crates/node/src/tee/remote_attestation.rs index 05e202a88f..c6124f1e66 100644 --- a/crates/node/src/tee/remote_attestation.rs +++ b/crates/node/src/tee/remote_attestation.rs @@ -24,10 +24,11 @@ use mpc_attestation::{ }; use near_mpc_contract_interface::types::{AllowedMpcDockerImageHash, Ed25519PublicKey}; use tee_authority::tee_authority::{AttestationError, TeeAuthority}; +use tokio_util::time::FutureExt; use mpc_primitives::hash::{LauncherDockerComposeHash, NodeImageHash}; use near_mpc_contract_interface::call_args as contract_args; -use tokio::sync::watch; +use tokio::{sync::watch, time::Instant}; const MIN_BACKOFF_DURATION: Duration = Duration::from_millis(100); const MAX_BACKOFF_DURATION: Duration = Duration::from_secs(60); @@ -63,26 +64,6 @@ pub struct AttestationSubmitter { pub attestation_reader: Arc, } -/// The stage a single iteration of [`periodic_attestation_submission`] is in. A cancelled -/// iteration cannot report anything itself, so it publishes each stage it enters, and the one -/// it was cancelled in ends up in the log and metric. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum RoundStage { - GenerateAttestation, - ReadExpiryBaseline, - SubmitAttestation, -} - -impl RoundStage { - fn as_label(self) -> &'static str { - match self { - RoundStage::GenerateAttestation => MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION, - RoundStage::ReadExpiryBaseline => MPC_TEE_ATTESTATION_STAGE_READ_EXPIRY_BASELINE, - RoundStage::SubmitAttestation => MPC_TEE_ATTESTATION_STAGE_SUBMIT_ATTESTATION, - } - } -} - /// Submits a [`contract_args::SubmitParticipantInfoArgs`] transaction containing the given /// participant's attestation and TLS public key, using the provided [`TransactionSender`] and /// retrying with backoff until [`TransactionStatus::Executed`] is observed. Retries are @@ -170,40 +151,31 @@ fn validate_remote_attestation( } impl AttestationSubmitter { - /// Generates a fresh attestation and submits it to the contract, publishing each stage it - /// enters to the given [`watch::Sender`] so a caller that drops this future can tell - /// which stage was cut off. Failures are logged, never propagated. - async fn generate_and_submit(&self, stage: &watch::Sender) { + async fn generate_attestation(&self) -> Option { let report_data: ReportData = ReportDataV1::new( *self.tls_public_key.as_bytes(), *self.account_public_key.as_bytes(), ) .into(); - stage.send_replace(RoundStage::GenerateAttestation); let result = self.tee_authority.generate_attestation(report_data).await; MPC_TEE_ATTESTATION_ATTEMPTS_TOTAL .with_label_values(&[outcome_label(result.is_ok())]) .inc(); - let attestation = match result { - Ok(attestation) => attestation, + match result { + Ok(attestation) => Some(attestation), Err(error @ AttestationError::CollateralFetch(_)) => { tracing::warn!(%error, "TEE attestation collateral fetch failed"); - return; + None } Err(error) => { tracing::error!(%error, "TEE attestation generation failed"); - return; + None } - }; - let allowed_image_hashes: Vec<_> = self - .allowed_image_hashes - .borrow() - .iter() - .map(|entry| entry.image_hash) - .collect(); - let allowed_launcher_compose_hashes = self.allowed_launcher_compose_hashes.borrow().clone(); - stage.send_replace(RoundStage::ReadExpiryBaseline); - let pre_submit_expiry = match self + } + } + + async fn read_expiry_baseline(&self) -> Option { + match self .attestation_reader .read_stored_attestation_expiry(&self.tls_public_key) .await @@ -218,7 +190,17 @@ impl AttestationSubmitter< ); None } - }; + } + } + + async fn submit_attestation(&self, attestation: Attestation, pre_submit_expiry: Option) { + let allowed_image_hashes: Vec<_> = self + .allowed_image_hashes + .borrow() + .iter() + .map(|entry| entry.image_hash) + .collect(); + let allowed_launcher_compose_hashes = self.allowed_launcher_compose_hashes.borrow().clone(); if let Err(error) = validate_remote_attestation( &attestation, self.tls_public_key.clone(), @@ -230,7 +212,6 @@ impl AttestationSubmitter< // fail on a stale view of the allowed hashes tracing::warn!("Attestation is not valid: {error}"); } - stage.send_replace(RoundStage::SubmitAttestation); let submission = submit_remote_attestation( self.tx_sender.clone(), attestation, @@ -262,37 +243,50 @@ where { let mut interval = tokio::time::interval(ATTESTATION_RESUBMISSION_INTERVAL); interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - periodic_attestation_submission(submitter, interval, ATTESTATION_RESUBMISSION_INTERVAL).await + periodic_attestation_submission(submitter, interval).await } -/// Periodically regenerates and submits this node's attestation. A round that times out is -/// abandoned, recorded in [`MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL`] under the stage it was -/// cut in, and the next tick starts over with a fresh attestation. Failures are logged, -/// never propagated; this task never returns. +/// Periodically regenerates and submits this node's attestation. A stage still running at the +/// round deadline is abandoned, recorded in [`MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL`], and +/// the next tick starts over with a fresh attestation. Failures are logged, never propagated; +/// this task never returns. #[tracing::instrument(skip_all)] async fn periodic_attestation_submission( submitter: AttestationSubmitter, mut interval_ticker: impl Tick, - round_timeout: Duration, ) { loop { interval_ticker.tick().await; - let (stage, _) = watch::channel(RoundStage::GenerateAttestation); - let round = tokio::time::timeout(round_timeout, submitter.generate_and_submit(&stage)); - if round.await.is_err() { - let stage = stage.borrow().as_label(); - MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL - .with_label_values(&[stage]) - .inc(); - tracing::warn!( - stage, - timeout = ?round_timeout, - "attestation round timed out; retrying from scratch on the next tick" - ); + let deadline = Instant::now() + ATTESTATION_RESUBMISSION_INTERVAL; + let Ok(generated) = submitter.generate_attestation().timeout_at(deadline).await else { + record_round_timeout(MPC_TEE_ATTESTATION_STAGE_GENERATE_ATTESTATION); + continue; + }; + let Some(attestation) = generated else { + continue; + }; + let Ok(pre_submit_expiry) = submitter.read_expiry_baseline().timeout_at(deadline).await + else { + record_round_timeout(MPC_TEE_ATTESTATION_STAGE_READ_EXPIRY_BASELINE); + continue; + }; + let submission = submitter.submit_attestation(attestation, pre_submit_expiry); + if submission.timeout_at(deadline).await.is_err() { + record_round_timeout(MPC_TEE_ATTESTATION_STAGE_SUBMIT_ATTESTATION); } } } +fn record_round_timeout(stage: &'static str) { + MPC_TEE_ATTESTATION_ROUND_TIMEOUTS_TOTAL + .with_label_values(&[stage]) + .inc(); + tracing::warn!( + stage, + "attestation round timed out; retrying from scratch on the next tick" + ); +} + #[cfg(test)] mod tests { use super::*; @@ -455,7 +449,6 @@ mod tests { tokio::spawn(periodic_attestation_submission( self.submitter.clone(), MockTicker::new(ticks), - ATTESTATION_RESUBMISSION_INTERVAL, )) } } @@ -529,7 +522,6 @@ mod tests { let MaybeReady::Future(parked_loop) = run_future_once(periodic_attestation_submission( setup.submitter.clone(), ticker.clone(), - ATTESTATION_RESUBMISSION_INTERVAL, )) else { panic!("the loop should park once its ticker runs out"); }; @@ -587,26 +579,6 @@ mod tests { handle.abort(); } - #[tokio::test(start_paused = true)] - #[expect(non_snake_case)] - async fn generate_and_submit__should_record_the_submission_stage_when_cut_off() { - // Given - let setup = test_setup(); - setup.sender().set_failing(true); - let (stage, _) = watch::channel(RoundStage::GenerateAttestation); - - // When - let round = tokio::time::timeout( - ATTESTATION_RESUBMISSION_INTERVAL, - setup.submitter.generate_and_submit(&stage), - ) - .await; - - // Then - assert!(round.is_err()); - assert_eq!(*stage.borrow(), RoundStage::SubmitAttestation); - } - async fn validate_locally_generated_attestation( config: LocalTeeAuthorityConfig, ) -> Result<(), VerificationError> {