From 06f00a2a2a3e5bf61329af346424b08026aa2b77 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Fri, 4 Sep 2026 11:54:07 -0300 Subject: [PATCH 1/2] feat(ntx-builder): materialize note eligibility in the database --- bin/ntx-builder/src/db/eligibility.rs | 251 ++++++++++++++++++ bin/ntx-builder/src/db/migrations.rs | 3 +- .../db/migrations/004_note_eligibility.sql | 10 + bin/ntx-builder/src/db/mod.rs | 32 ++- .../src/db/queries/available_notes/mod.rs | 174 +----------- .../db/queries/discard_notes/discard_note.sql | 5 +- .../src/db/queries/discard_notes/mod.rs | 21 +- .../insert_network_note.sql | 7 +- .../db/queries/insert_network_notes/mod.rs | 27 +- bin/ntx-builder/src/db/queries/mod.rs | 16 +- .../src/db/queries/notes_failed/mod.rs | 44 ++- .../db/queries/notes_failed/note_failed.sql | 9 +- .../notes_failed/select_note_backoff.sql | 3 + .../db/queries/reset_sponsored_notes/mod.rs | 25 ++ .../reset_sponsored_notes.sql | 6 + bin/ntx-builder/src/db/queries/tests.rs | 150 +++++++++++ bin/ntx-builder/src/test_utils.rs | 20 +- 17 files changed, 604 insertions(+), 199 deletions(-) create mode 100644 bin/ntx-builder/src/db/eligibility.rs create mode 100644 bin/ntx-builder/src/db/migrations/004_note_eligibility.sql create mode 100644 bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql create mode 100644 bin/ntx-builder/src/db/queries/reset_sponsored_notes/mod.rs create mode 100644 bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql diff --git a/bin/ntx-builder/src/db/eligibility.rs b/bin/ntx-builder/src/db/eligibility.rs new file mode 100644 index 0000000000..3032dfc35d --- /dev/null +++ b/bin/ntx-builder/src/db/eligibility.rs @@ -0,0 +1,251 @@ +//! When a network note becomes eligible for a transaction attempt. +//! +//! Two things delay a note: its execution hint, which opens a window of consumable blocks, and the +//! exponential backoff applied after a failed attempt. This module computes both, and every write +//! path that touches a note stores the result in `notes.next_eligible_block` so the scheduler can +//! ask for the ready accounts with a single indexed query. + +use miden_protocol::block::BlockNumber; +use miden_standards::note::NoteExecutionHint; + +/// Block number stored for a note that can never become eligible again. +pub const NEVER_ELIGIBLE: BlockNumber = BlockNumber::MAX; + +/// Returns the block at which a freshly ingested note becomes eligible. +pub fn first_eligible_block(hint: NoteExecutionHint, created_at: BlockNumber) -> BlockNumber { + eligible_at_or_after(hint, created_at) +} + +/// Returns the block at which a note becomes eligible again after `attempts` failed attempts, the +/// latest of which was recorded at `last_attempt`. +pub fn eligible_block_after_failure( + hint: NoteExecutionHint, + attempts: usize, + last_attempt: BlockNumber, +) -> BlockNumber { + eligible_at_or_after(hint, backoff_ready_block(Some(last_attempt), attempts)) +} + +/// Returns the first block at or after `floor` at which the note's execution hint permits +/// consumption. +fn eligible_at_or_after(hint: NoteExecutionHint, floor: BlockNumber) -> BlockNumber { + hint_next_consumable_block(hint, floor).map_or(floor, |block| block.max(floor)) +} + +/// Checks if the backoff block period has passed. +#[expect(clippy::cast_precision_loss, clippy::cast_sign_loss)] +pub fn has_backoff_passed( + chain_tip: BlockNumber, + last_attempt: Option, + attempts: usize, +) -> bool { + if attempts == 0 { + return true; + } + let blocks_passed = last_attempt + .and_then(|last| chain_tip.checked_sub(last.as_u32())) + .unwrap_or_default(); + + let backoff_threshold = (0.25 * attempts as f64).exp().round() as usize; + + blocks_passed.as_usize() > backoff_threshold +} + +/// Returns the first block at which a note's backoff period elapses. +#[expect( + clippy::cast_precision_loss, + clippy::cast_sign_loss, + clippy::cast_possible_truncation +)] +pub fn backoff_ready_block(last_attempt: Option, attempts: usize) -> BlockNumber { + if attempts == 0 { + return last_attempt.unwrap_or(BlockNumber::GENESIS); + } + let last = last_attempt.unwrap_or(BlockNumber::GENESIS); + let threshold = (0.25 * attempts as f64).exp().round() as u32; + last + threshold + 1 +} + +/// Returns the earliest block worth re-checking a currently-ineligible note at. +pub fn note_recheck_block( + hint: NoteExecutionHint, + chain_tip: BlockNumber, + last_attempt: Option, + attempts: usize, + backoff_ok: bool, + hint_ok: bool, +) -> BlockNumber { + let mut recheck = chain_tip.child(); + if !backoff_ok { + recheck = recheck.max(backoff_ready_block(last_attempt, attempts)); + } + if !hint_ok && let Some(hint_block) = hint_next_consumable_block(hint, chain_tip) { + recheck = recheck.max(hint_block); + } + recheck +} + +/// Returns the first block at or after `from` for which `hint.can_be_consumed` turns true, or +/// `None` when the hint imposes no future-block constraint ([`NoteExecutionHint::None`], `Always` +/// or `Unknown`), leaving the caller's floor in place. +pub fn hint_next_consumable_block( + hint: NoteExecutionHint, + from: BlockNumber, +) -> Option { + match hint { + NoteExecutionHint::None | NoteExecutionHint::Always | NoteExecutionHint::Unknown(_) => None, + NoteExecutionHint::AfterBlock { block_num } => Some(block_num), + NoteExecutionHint::OnBlockSlot { round_len, slot_len, slot_offset } => { + let block = u64::from(from.as_u32()); + // `1 << round_len` as `can_be_consumed` computes it, in u64 to avoid the overflow its + // u32 shift would hit; bail to the caller's floor for degenerate exponents. + let round_len_blocks = 1u64.checked_shl(u32::from(round_len))?; + let slot_len_blocks = 1u64.checked_shl(u32::from(slot_len))?; + let round_index = block / round_len_blocks; + let slot_start = + round_index * round_len_blocks + u64::from(slot_offset) * slot_len_blocks; + let slot_end = slot_start + slot_len_blocks; + let next = if block < slot_start { + slot_start + } else if block >= slot_end { + // Past this round's slot; the next opening is the same slot one round later. + slot_start + round_len_blocks + } else { + block + }; + // Beyond the representable block range the note is effectively never consumable; clamp + // so the caller schedules at most a far-future recheck rather than wrapping. + Some(BlockNumber::from(u32::try_from(next).unwrap_or(u32::MAX))) + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Brute-forces the first block at or after `from` for which the hint is consumable, by + /// scanning forward. Used as an independent oracle for [`hint_next_consumable_block`]. + fn brute_force_next(hint: NoteExecutionHint, from: u32) -> Option { + (from..=from.saturating_add(4096)) + .find(|&b| hint.can_be_consumed(BlockNumber::from(b)) == Some(true)) + } + + /// [`hint_next_consumable_block`] must agree, block for block, with scanning + /// [`NoteExecutionHint::can_be_consumed`] forward. This guards against the slot arithmetic + /// drifting from the protocol definition it mirrors. + #[test] + fn hint_next_consumable_block_matches_can_be_consumed() { + let hints = [ + NoteExecutionHint::after_block(BlockNumber::from(200)), + NoteExecutionHint::on_block_slot(10, 7, 1), + NoteExecutionHint::on_block_slot(8, 4, 0), + NoteExecutionHint::on_block_slot(9, 5, 3), + ]; + for hint in hints { + for b in 0u32..1300 { + // Only meaningful while the note is currently NOT consumable. + if hint.can_be_consumed(BlockNumber::from(b)) != Some(false) { + continue; + } + let got = hint_next_consumable_block(hint, BlockNumber::from(b)) + .expect("a windowed hint must report a next block") + .as_u32(); + let expected = brute_force_next(hint, b) + .expect("oracle must find a consumable block within the scan window"); + assert_eq!(got, expected, "hint {hint:?} at block {b}"); + } + } + } + + #[rstest::rstest] + #[test] + #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] + #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] + #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] + #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] + #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] + #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] + #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] + #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] + #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] + #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] + fn backoff_has_passed( + #[case] last_attempt_block_num: Option, + #[case] current_block_num: BlockNumber, + #[case] attempt_count: usize, + #[case] backoff_should_have_passed: bool, + ) { + assert_eq!( + backoff_should_have_passed, + has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) + ); + } + + /// The block stored after a failure is exactly the first block at which the read-time backoff + /// check passes. This is what lets the stored column stand in for the check. + #[rstest::rstest] + #[test] + #[case(1)] + #[case(3)] + #[case(10)] + #[case(20)] + fn stored_block_after_failure_matches_the_backoff_check(#[case] attempts: usize) { + let last_attempt = BlockNumber::from(100); + let stored = + eligible_block_after_failure(NoteExecutionHint::Always, attempts, last_attempt); + + assert!( + has_backoff_passed(stored, Some(last_attempt), attempts), + "the stored block must satisfy the backoff check", + ); + assert!( + !has_backoff_passed( + stored.parent().expect("the stored block is past genesis"), + Some(last_attempt), + attempts + ), + "no earlier block may satisfy it, or the stored value would hide the note", + ); + } + + /// A hint window that opens later than the backoff wins, and vice versa: the note is eligible + /// only once both allow it. + #[test] + fn stored_block_takes_the_later_of_backoff_and_hint() { + let last_attempt = BlockNumber::from(10); + + let hint = NoteExecutionHint::after_block(BlockNumber::from(500)); + assert_eq!(eligible_block_after_failure(hint, 1, last_attempt), BlockNumber::from(500),); + + let hint = NoteExecutionHint::after_block(BlockNumber::from(1)); + assert_eq!( + eligible_block_after_failure(hint, 1, last_attempt), + backoff_ready_block(Some(last_attempt), 1), + ); + } + + /// An ingested note is eligible immediately unless its hint says otherwise. + #[test] + fn first_eligible_block_follows_the_hint() { + let created_at = BlockNumber::from(42); + + assert_eq!( + first_eligible_block(NoteExecutionHint::Always, created_at), + created_at, + "an unconstrained note is eligible in the block that created it", + ); + assert_eq!( + first_eligible_block( + NoteExecutionHint::after_block(BlockNumber::from(100)), + created_at + ), + BlockNumber::from(100), + ); + assert_eq!( + first_eligible_block(NoteExecutionHint::after_block(BlockNumber::from(7)), created_at), + created_at, + "a window that already opened does not move the note into the past", + ); + } +} diff --git a/bin/ntx-builder/src/db/migrations.rs b/bin/ntx-builder/src/db/migrations.rs index 84e38a6132..6b3277780e 100644 --- a/bin/ntx-builder/src/db/migrations.rs +++ b/bin/ntx-builder/src/db/migrations.rs @@ -67,10 +67,11 @@ mod tests { use super::*; - const EXPECTED_SCHEMA_HASHES: [SchemaHash; 3] = [ + const EXPECTED_SCHEMA_HASHES: [SchemaHash; 4] = [ SchemaHash::from_hex("c631b773787903a3dd5ea4df5e7374119b3f02b35bacf14d11eacd8d8500e3d9"), SchemaHash::from_hex("26b17298444f674b06327ae7289516fe75b59926741b1221ebf36735822d116a"), SchemaHash::from_hex("6f27c48c71d173366c90752c330bf888332923e68a290ac3acdb5861539120e8"), + SchemaHash::from_hex("638b3991fe1b025ab8820e5cfc902d82d212a6bd3eb26d29af3959de1487ea5c"), ]; #[test] diff --git a/bin/ntx-builder/src/db/migrations/004_note_eligibility.sql b/bin/ntx-builder/src/db/migrations/004_note_eligibility.sql new file mode 100644 index 0000000000..f39f24d448 --- /dev/null +++ b/bin/ntx-builder/src/db/migrations/004_note_eligibility.sql @@ -0,0 +1,10 @@ +-- Materializes note eligibility so the scheduler can ask for the ready accounts. +ALTER TABLE notes ADD COLUMN next_eligible_block BIGINT NOT NULL DEFAULT 0 + CHECK (next_eligible_block BETWEEN 0 AND 0xFFFFFFFF); + +-- Replaces the account-only partial index with one that also covers the eligibility filter and the +-- attempt budget, so the ready-accounts query is answered from the index. +DROP INDEX idx_notes_account_pending; +CREATE INDEX idx_notes_account_pending + ON notes(account_id, next_eligible_block, attempt_count) + WHERE committed_at IS NULL; diff --git a/bin/ntx-builder/src/db/mod.rs b/bin/ntx-builder/src/db/mod.rs index cdda3806ae..b55a395da5 100644 --- a/bin/ntx-builder/src/db/mod.rs +++ b/bin/ntx-builder/src/db/mod.rs @@ -4,6 +4,8 @@ use std::path::{Path, PathBuf}; use anyhow::Context; use miden_node_db::DatabaseError; +#[cfg(test)] +use miden_node_db::SqlTypeConvert; use miden_node_db::sqlite::{DbReader, DbWriter}; use miden_node_tracing::{info, miden_instrument}; use miden_protocol::Word; @@ -25,6 +27,7 @@ use crate::db::queries::NoteStatusRow; use crate::sponsorship::SponsorshipNote; use crate::{COMPONENT, NoteError, db}; +pub(crate) mod eligibility; pub(crate) mod queries; mod migrations; @@ -434,6 +437,21 @@ impl NtxDbReader { .unwrap() } + /// Reads the stored eligibility block of a note, so tests can assert that the write paths + /// materialize exactly what [`eligibility`] computes. + pub(crate) async fn note_eligibility(&self, note_id: NoteId) -> Option { + self.reader + .read("note_eligibility", move |tx| { + let sql = "SELECT next_eligible_block FROM notes WHERE note_id = ?1"; + Ok::, DatabaseError>( + tx.query(sql, &[¬e_id], |row| row.get::(0))?.into_iter().next(), + ) + }) + .await + .unwrap() + .map(|block| BlockNumber::from_raw_sql(block).unwrap()) + } + pub(crate) async fn count_notes(&self) -> i64 { self.count("SELECT COUNT(*) FROM notes").await } @@ -473,12 +491,24 @@ impl NtxDbWriter { .await } + /// Inserts notes as if they were created in the genesis block, so their stored eligibility is + /// driven by their execution hint alone. pub(crate) async fn insert_network_notes( &self, notes: Vec, + ) -> Result<(), DatabaseError> { + self.insert_network_notes_at(notes, BlockNumber::GENESIS).await + } + + pub(crate) async fn insert_network_notes_at( + &self, + notes: Vec, + created_at: BlockNumber, ) -> Result<(), DatabaseError> { self.writer - .write("insert_network_notes", move |tx| queries::insert_network_notes(tx, ¬es)) + .write("insert_network_notes", move |tx| { + queries::insert_network_notes(tx, ¬es, created_at) + }) .await } diff --git a/bin/ntx-builder/src/db/queries/available_notes/mod.rs b/bin/ntx-builder/src/db/queries/available_notes/mod.rs index 3ac11541c0..3bdb570c74 100644 --- a/bin/ntx-builder/src/db/queries/available_notes/mod.rs +++ b/bin/ntx-builder/src/db/queries/available_notes/mod.rs @@ -5,7 +5,9 @@ use miden_node_db::{DatabaseError, SqlTypeConvert}; use miden_protocol::account::AccountId; use miden_protocol::block::BlockNumber; use miden_protocol::note::Note; -use miden_standards::note::{AccountTargetNetworkNote, NoteExecutionHint}; +use miden_standards::note::AccountTargetNetworkNote; + +use crate::db::eligibility::{has_backoff_passed, note_recheck_block}; const SQL: &str = include_str!("available_notes.sql"); @@ -71,173 +73,3 @@ pub fn available_notes( Ok(AvailableNotes { eligible, next_retry_block }) } - -// HELPERS -// ================================================================================================ - -/// Checks if the backoff block period has passed. -/// -/// The number of blocks passed since the last attempt must be greater than or equal to -/// e^(0.25 * `attempt_count`) rounded to the nearest integer. -#[expect(clippy::cast_precision_loss, clippy::cast_sign_loss)] -fn has_backoff_passed( - chain_tip: BlockNumber, - last_attempt: Option, - attempts: usize, -) -> bool { - if attempts == 0 { - return true; - } - let blocks_passed = last_attempt - .and_then(|last| chain_tip.checked_sub(last.as_u32())) - .unwrap_or_default(); - - let backoff_threshold = (0.25 * attempts as f64).exp().round() as usize; - - blocks_passed.as_usize() > backoff_threshold -} - -/// Returns the first block at which a note's backoff period elapses. -/// -/// Inverts [`has_backoff_passed`], which is satisfied once `chain_tip - last_attempt` exceeds the -/// threshold, so the first eligible tip is `last_attempt + threshold + 1`. Only meaningful when the -/// note has been attempted (`attempts > 0`); for an unattempted note backoff is always passed. -#[expect( - clippy::cast_precision_loss, - clippy::cast_sign_loss, - clippy::cast_possible_truncation -)] -fn backoff_ready_block(last_attempt: Option, attempts: usize) -> BlockNumber { - let last = last_attempt.unwrap_or(BlockNumber::GENESIS); - let threshold = (0.25 * attempts as f64).exp().round() as u32; - last + threshold + 1 -} - -/// Returns the earliest block worth re-checking a currently-ineligible note at. -/// -/// The result is at least the next block (so it always lies in the future) and accounts for the -/// reasons the note is ineligible: backoff is inverted exactly via [`backoff_ready_block`], and the -/// execution-hint window is inverted exactly via [`hint_next_consumable_block`]. Inverting the hint -/// exactly (rather than re-checking every block) lets an actor with only a window-pending note wait -/// for that block and idle-deactivate in between, instead of querying the DB on every block. -fn note_recheck_block( - hint: NoteExecutionHint, - chain_tip: BlockNumber, - last_attempt: Option, - attempts: usize, - backoff_ok: bool, - hint_ok: bool, -) -> BlockNumber { - let mut recheck = chain_tip.child(); - if !backoff_ok { - recheck = recheck.max(backoff_ready_block(last_attempt, attempts)); - } - if !hint_ok && let Some(hint_block) = hint_next_consumable_block(hint, chain_tip) { - recheck = recheck.max(hint_block); - } - recheck -} - -/// Returns the first block at or after `from` for which `hint.can_be_consumed` turns true, or `None` -/// when the hint imposes no future-block constraint ([`NoteExecutionHint::None`]/`Always`). -/// -/// This is the exact inverse of [`NoteExecutionHint::can_be_consumed`]: `AfterBlock` opens at its -/// block, and the periodic `OnBlockSlot` window opens either later this round (if `from` precedes -/// the slot) or at the same slot in the next round (if `from` is past it). The slot arithmetic -/// mirrors `can_be_consumed`; the `tests` module cross-checks the two against each other. -/// Degenerate round/slot exponents that would overflow are treated as "no exact answer" (`None`), -/// leaving the caller's next-block default. -fn hint_next_consumable_block(hint: NoteExecutionHint, from: BlockNumber) -> Option { - match hint { - NoteExecutionHint::None | NoteExecutionHint::Always | NoteExecutionHint::Unknown(_) => None, - NoteExecutionHint::AfterBlock { block_num } => Some(block_num), - NoteExecutionHint::OnBlockSlot { round_len, slot_len, slot_offset } => { - let block = u64::from(from.as_u32()); - // `1 << round_len` as `can_be_consumed` computes it, in u64 to avoid the overflow its - // u32 shift would hit; bail to the next-block default for degenerate exponents. - let round_len_blocks = 1u64.checked_shl(u32::from(round_len))?; - let slot_len_blocks = 1u64.checked_shl(u32::from(slot_len))?; - let round_index = block / round_len_blocks; - let slot_start = - round_index * round_len_blocks + u64::from(slot_offset) * slot_len_blocks; - let slot_end = slot_start + slot_len_blocks; - let next = if block < slot_start { - slot_start - } else if block >= slot_end { - // Past this round's slot; the next opening is the same slot one round later. - slot_start + round_len_blocks - } else { - block - }; - // Beyond the representable block range the note is effectively never consumable; clamp - // so the caller schedules at most a far-future recheck rather than wrapping. - Some(BlockNumber::from(u32::try_from(next).unwrap_or(u32::MAX))) - }, - } -} - -#[cfg(test)] -mod tests { - use miden_protocol::block::BlockNumber; - use miden_standards::note::NoteExecutionHint; - - use super::{has_backoff_passed, hint_next_consumable_block}; - - /// Brute-forces the first block at or after `from` for which the hint is consumable, by - /// scanning forward. Used as an independent oracle for [`hint_next_consumable_block`]. - fn brute_force_next(hint: NoteExecutionHint, from: u32) -> Option { - (from..=from.saturating_add(4096)) - .find(|&b| hint.can_be_consumed(BlockNumber::from(b)) == Some(true)) - } - - /// [`hint_next_consumable_block`] must agree, block for block, with scanning - /// [`NoteExecutionHint::can_be_consumed`] forward. This guards against the slot arithmetic - /// drifting from the protocol definition it mirrors. - #[test] - fn hint_next_consumable_block_matches_can_be_consumed() { - let hints = [ - NoteExecutionHint::after_block(BlockNumber::from(200)), - NoteExecutionHint::on_block_slot(10, 7, 1), // blocks 128..256, 1152..1280, ... - NoteExecutionHint::on_block_slot(8, 4, 0), // blocks 0..16, 256..272, ... - NoteExecutionHint::on_block_slot(9, 5, 3), - ]; - for hint in hints { - for b in 0u32..1300 { - // Only meaningful while the note is currently NOT consumable. - if hint.can_be_consumed(BlockNumber::from(b)) != Some(false) { - continue; - } - let got = hint_next_consumable_block(hint, BlockNumber::from(b)) - .expect("a windowed hint must report a next block") - .as_u32(); - let expected = brute_force_next(hint, b) - .expect("oracle must find a consumable block within the scan window"); - assert_eq!(got, expected, "hint {hint:?} at block {b}"); - } - } - } - - #[rstest::rstest] - #[test] - #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] - #[case::no_attempts(None, BlockNumber::GENESIS, 0, true)] - #[case::one_attempt(Some(BlockNumber::GENESIS), BlockNumber::from(2), 1, true)] - #[case::three_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(3), 3, true)] - #[case::ten_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(13), 10, true)] - #[case::twenty_attempts(Some(BlockNumber::GENESIS), BlockNumber::from(149), 20, true)] - #[case::one_attempt_false(Some(BlockNumber::GENESIS), BlockNumber::from(1), 1, false)] - #[case::three_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(2), 3, false)] - #[case::ten_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(12), 10, false)] - #[case::twenty_attempts_false(Some(BlockNumber::GENESIS), BlockNumber::from(148), 20, false)] - fn backoff_has_passed( - #[case] last_attempt_block_num: Option, - #[case] current_block_num: BlockNumber, - #[case] attempt_count: usize, - #[case] backoff_should_have_passed: bool, - ) { - assert_eq!( - backoff_should_have_passed, - has_backoff_passed(current_block_num, last_attempt_block_num, attempt_count) - ); - } -} diff --git a/bin/ntx-builder/src/db/queries/discard_notes/discard_note.sql b/bin/ntx-builder/src/db/queries/discard_notes/discard_note.sql index d37f3ff178..477ab3713f 100644 --- a/bin/ntx-builder/src/db/queries/discard_notes/discard_note.sql +++ b/bin/ntx-builder/src/db/queries/discard_notes/discard_note.sql @@ -1,5 +1,8 @@ -- Marks a note as permanently unconsumable by pinning `attempt_count` to `max_attempts`, recording -- the block at which it was discarded in `last_attempt`, and storing the reason in `last_error`. +-- +-- `next_eligible_block` is pinned to the maximum block number as well, so the note is excluded by +-- the eligibility filter even if the attempt cap is later raised. UPDATE notes -SET attempt_count = ?2, last_attempt = ?3, last_error = ?4 +SET attempt_count = ?2, last_attempt = ?3, last_error = ?4, next_eligible_block = ?5 WHERE nullifier = ?1 diff --git a/bin/ntx-builder/src/db/queries/discard_notes/mod.rs b/bin/ntx-builder/src/db/queries/discard_notes/mod.rs index eab9fab015..4b734558f0 100644 --- a/bin/ntx-builder/src/db/queries/discard_notes/mod.rs +++ b/bin/ntx-builder/src/db/queries/discard_notes/mod.rs @@ -5,14 +5,16 @@ use miden_node_db::{DatabaseError, SqlTypeConvert}; use miden_protocol::block::BlockNumber; use miden_protocol::note::Nullifier; +use crate::db::eligibility::NEVER_ELIGIBLE; + const SQL: &str = include_str!("discard_note.sql"); -/// Marks notes as permanently unconsumable by pinning `attempt_count` to `max_attempts`. +/// Marks notes as permanently unconsumable by pinning `attempt_count` to `max_attempts` and +/// `next_eligible_block` to [`NEVER_ELIGIBLE`]. /// /// A note whose own consumption exceeds the per-transaction cycle budget can never be consumed in -/// any transaction, so retrying it is pointless. Setting `attempt_count` to `max_attempts` takes it -/// out of the pending set immediately (`available_notes`/`account_has_pending_notes` filter on -/// `attempt_count < max_attempts`) and makes +/// any transaction, so retrying it is pointless. Both pinned columns take it out of the pending set +/// immediately (`available_notes` and `ready_accounts` filter on each of them) and make /// [`get_note_status`](super::get_note_status) derive it as `Discarded`, while `last_error` records /// why. #[expect(clippy::cast_possible_wrap)] @@ -26,7 +28,16 @@ pub fn discard_notes( let block_num_val = block_num.to_raw_sql(); let reason = reason.to_string(); for nullifier in nullifiers { - tx.execute(SQL, &[nullifier, &(max_attempts as i64), &block_num_val, &reason])?; + tx.execute( + SQL, + &[ + nullifier, + &(max_attempts as i64), + &block_num_val, + &reason, + &NEVER_ELIGIBLE.to_raw_sql(), + ], + )?; } Ok(()) } diff --git a/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql b/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql index 6107e97470..0efdc48581 100644 --- a/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql +++ b/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql @@ -2,5 +2,8 @@ -- block (e.g. on a redelivery from the subscription stream) is a no-op rather than a constraint -- violation. `attempt_count` defaults to 0 and the remaining backoff/lifecycle columns default to -- NULL. -INSERT OR IGNORE INTO notes (nullifier, account_id, note_data, note_id) -VALUES (?1, ?2, ?3, ?4) +-- +-- `next_eligible_block` is the first block at which the note's execution hint permits consumption. +-- A note that has never been attempted has no backoff, so the hint is the only delay. +INSERT OR IGNORE INTO notes (nullifier, account_id, note_data, note_id, next_eligible_block) +VALUES (?1, ?2, ?3, ?4, ?5) diff --git a/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs b/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs index dd0acf7ffb..4856b8eb62 100644 --- a/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs +++ b/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs @@ -1,21 +1,38 @@ //! Inserts network notes from a committed block. -use miden_node_db::DatabaseError; use miden_node_db::sqlite::WriteTx; +use miden_node_db::{DatabaseError, SqlTypeConvert}; +use miden_protocol::block::BlockNumber; use miden_standards::note::AccountTargetNetworkNote; +use crate::db::eligibility::first_eligible_block; + const SQL: &str = include_str!("insert_network_note.sql"); -/// Inserts network notes from a committed block. Uses `INSERT OR IGNORE` so re-applying the same -/// block (e.g. on a redelivery from the subscription stream) is a no-op rather than a constraint -/// violation. +/// Inserts network notes created by the block at `created_at`. Uses `INSERT OR IGNORE` so +/// re-applying the same block (e.g. on a redelivery from the subscription stream) is a no-op rather +/// than a constraint violation. +/// +/// Each note's `next_eligible_block` is derived from its execution hint, so a note inside a future +/// window is not selected before the window opens. pub fn insert_network_notes( tx: &WriteTx<'_>, notes: &[AccountTargetNetworkNote], + created_at: BlockNumber, ) -> Result<(), DatabaseError> { for note in notes { let inner = note.as_note(); - tx.execute(SQL, &[&inner.nullifier(), ¬e.target_account_id(), inner, &inner.id()])?; + let eligible_from = first_eligible_block(note.execution_hint(), created_at); + tx.execute( + SQL, + &[ + &inner.nullifier(), + ¬e.target_account_id(), + inner, + &inner.id(), + &eligible_from.to_raw_sql(), + ], + )?; } Ok(()) } diff --git a/bin/ntx-builder/src/db/queries/mod.rs b/bin/ntx-builder/src/db/queries/mod.rs index 9b6502546c..37d0ec8356 100644 --- a/bin/ntx-builder/src/db/queries/mod.rs +++ b/bin/ntx-builder/src/db/queries/mod.rs @@ -71,6 +71,9 @@ pub use mark_sponsorships_consumed::mark_sponsorships_consumed; mod notes_failed; pub use notes_failed::notes_failed; +mod reset_sponsored_notes; +pub use reset_sponsored_notes::reset_sponsored_notes; + mod select_chain_state; pub use select_chain_state::select_chain_state; @@ -163,15 +166,18 @@ pub fn apply_committed_block( } } - insert_network_notes(tx, &effects.network_notes)?; + let block_num = effects.header.block_num(); + + insert_network_notes(tx, &effects.network_notes, block_num)?; insert_sponsorship_notes(tx, &effects.sponsorship_notes)?; - mark_notes_consumed(tx, &effects.nullifiers, effects.header.block_num())?; - mark_sponsorships_consumed(tx, &effects.nullifiers, effects.header.block_num())?; + mark_notes_consumed(tx, &effects.nullifiers, block_num)?; + mark_sponsorships_consumed(tx, &effects.nullifiers, block_num)?; - // Resolved after the consumption marks so a feature note consumed in this same block does not - // produce a wakeup. + // Both of these run after the consumption marks so a feature note consumed in this same block + // is neither woken nor made eligible again. let sponsored = get_target_account_ids_for_sponsor_notes(tx, &effects.sponsorship_notes)?; + reset_sponsored_notes(tx, &effects.sponsorship_notes, block_num)?; update_chain_state_tip(tx, effects.header.block_num(), &effects.header, chain_mmr)?; diff --git a/bin/ntx-builder/src/db/queries/notes_failed/mod.rs b/bin/ntx-builder/src/db/queries/notes_failed/mod.rs index 7fcddb06e7..5fd78f9d50 100644 --- a/bin/ntx-builder/src/db/queries/notes_failed/mod.rs +++ b/bin/ntx-builder/src/db/queries/notes_failed/mod.rs @@ -4,14 +4,17 @@ use miden_node_db::sqlite::WriteTx; use miden_node_db::{DatabaseError, SqlTypeConvert}; use miden_node_tracing::ErrorReport; use miden_protocol::block::BlockNumber; -use miden_protocol::note::Nullifier; +use miden_protocol::note::{Note, Nullifier}; +use miden_standards::note::AccountTargetNetworkNote; use crate::NoteError; +use crate::db::eligibility::eligible_block_after_failure; const SQL: &str = include_str!("note_failed.sql"); +const SELECT_BACKOFF_SQL: &str = include_str!("select_note_backoff.sql"); -/// Marks notes as failed by incrementing `attempt_count`, setting `last_attempt`, and storing the -/// latest error message. +/// Marks notes as failed by incrementing `attempt_count`, setting `last_attempt`, storing the +/// latest error message, and moving `next_eligible_block` to the end of the new backoff window. pub fn notes_failed( tx: &WriteTx<'_>, failed_notes: &[(Nullifier, NoteError)], @@ -20,8 +23,41 @@ pub fn notes_failed( let block_num_val = block_num.to_raw_sql(); for (nullifier, error) in failed_notes { + let Some(eligible_from) = next_eligible_block(tx, nullifier, block_num)? else { + // The note is gone, so there is nothing to penalize. + continue; + }; let error_report = error.as_report(); - tx.execute(SQL, &[nullifier, &block_num_val, &error_report])?; + tx.execute(SQL, &[nullifier, &block_num_val, &error_report, &eligible_from.to_raw_sql()])?; } Ok(()) } + +/// Returns the eligibility block to store for a note that is failing now, or `None` when the note +/// has no row. +fn next_eligible_block( + tx: &WriteTx<'_>, + nullifier: &Nullifier, + block_num: BlockNumber, +) -> Result, DatabaseError> { + #[expect(clippy::cast_sign_loss)] + let row = tx + .query(SELECT_BACKOFF_SQL, &[nullifier], |row| { + Ok((row.get::(0)? as usize, row.get::(1)?)) + })? + .into_iter() + .next(); + + let Some((attempt_count, note)) = row else { + return Ok(None); + }; + let note = AccountTargetNetworkNote::new(note).map_err(|source| { + DatabaseError::deserialization("failed to convert to network note", source) + })?; + + Ok(Some(eligible_block_after_failure( + note.execution_hint(), + attempt_count + 1, + block_num, + ))) +} diff --git a/bin/ntx-builder/src/db/queries/notes_failed/note_failed.sql b/bin/ntx-builder/src/db/queries/notes_failed/note_failed.sql index ccce5d0923..e17bb61d9c 100644 --- a/bin/ntx-builder/src/db/queries/notes_failed/note_failed.sql +++ b/bin/ntx-builder/src/db/queries/notes_failed/note_failed.sql @@ -1,5 +1,8 @@ --- Marks a note as failed by incrementing `attempt_count`, setting `last_attempt`, and storing the --- latest error message. +-- Marks a note as failed by incrementing `attempt_count`, setting `last_attempt`, storing the +-- latest error message, and moving `next_eligible_block` to the end of the new backoff window. UPDATE notes -SET attempt_count = attempt_count + 1, last_attempt = ?2, last_error = ?3 +SET attempt_count = attempt_count + 1, + last_attempt = ?2, + last_error = ?3, + next_eligible_block = ?4 WHERE nullifier = ?1 diff --git a/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql b/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql new file mode 100644 index 0000000000..97e0998ac5 --- /dev/null +++ b/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql @@ -0,0 +1,3 @@ +-- Reads what the new eligibility block of a failing note depends on: its current attempt count and +-- its execution hint (carried by the serialized note). +SELECT attempt_count, note_data FROM notes WHERE nullifier = ?1 diff --git a/bin/ntx-builder/src/db/queries/reset_sponsored_notes/mod.rs b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/mod.rs new file mode 100644 index 0000000000..15cc022b18 --- /dev/null +++ b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/mod.rs @@ -0,0 +1,25 @@ +//! Makes the feature notes that just gained a sponsorship eligible again. + +use miden_node_db::sqlite::{InList, WriteTx}; +use miden_node_db::{DatabaseError, SqlTypeConvert}; +use miden_protocol::block::BlockNumber; + +use crate::sponsorship::SponsorshipNote; + +const SQL: &str = include_str!("reset_sponsored_notes.sql"); + +/// Clears the backoff of every pending feature note the given sponsorships are bound to, so it is +/// eligible at `block_num`. Returns the number of notes made eligible. +pub fn reset_sponsored_notes( + tx: &WriteTx<'_>, + sponsorships: &[SponsorshipNote], + block_num: BlockNumber, +) -> Result { + if sponsorships.is_empty() { + return Ok(0); + } + let feature_note_ids = + InList::from_values(sponsorships.iter().map(SponsorshipNote::feature_note_id)); + + tx.execute(SQL, &[&feature_note_ids, &block_num.to_raw_sql()]) +} diff --git a/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql new file mode 100644 index 0000000000..2ea507f5b0 --- /dev/null +++ b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql @@ -0,0 +1,6 @@ +-- Makes the pending feature notes that just gained a `FEE_SPONSORSHIP` note eligible again. + +UPDATE notes +SET next_eligible_block = ?2 +WHERE committed_at IS NULL + AND note_id IN (SELECT value FROM rarray(?1)) diff --git a/bin/ntx-builder/src/db/queries/tests.rs b/bin/ntx-builder/src/db/queries/tests.rs index c8a211f974..371cb3733c 100644 --- a/bin/ntx-builder/src/db/queries/tests.rs +++ b/bin/ntx-builder/src/db/queries/tests.rs @@ -12,9 +12,11 @@ use miden_protocol::block::BlockNumber; use miden_protocol::crypto::merkle::mmr::PartialMmr; use miden_protocol::note::NoteId; use miden_protocol::transaction::TransactionId; +use miden_standards::note::NoteExecutionHint; use crate::NoteError; use crate::committed_block::CommittedBlockEffects; +use crate::db::eligibility::{NEVER_ELIGIBLE, eligible_block_after_failure}; use crate::db::test_setup; use crate::sponsorship::SponsorshipNote; use crate::test_utils::*; @@ -287,6 +289,154 @@ async fn apply_committed_block_returns_sponsored_account_wakeups() { ); } +// NOTE ELIGIBILITY +// ================================================================================================ +// +// Every write path that touches a note must store exactly what `db::eligibility` computes. These +// tests compare the stored column against the helpers, which is what lets the column stand in for +// the read-time check. + +#[tokio::test] +async fn ingestion_stores_the_hint_derived_eligibility() { + let (db, _dir) = test_setup().await; + let account_id = mock_network_account_id(); + let created_at = BlockNumber::from(40); + + let unconstrained = mock_single_target_note(account_id, 1); + let windowed = mock_single_target_note_with_hint( + account_id, + 2, + NoteExecutionHint::after_block(BlockNumber::from(100)), + ); + let past_window = mock_single_target_note_with_hint( + account_id, + 3, + NoteExecutionHint::after_block(BlockNumber::from(7)), + ); + + db.insert_network_notes_at( + vec![unconstrained.clone(), windowed.clone(), past_window.clone()], + created_at, + ) + .await + .unwrap(); + + assert_eq!( + db.note_eligibility(unconstrained.as_note().id()).await, + Some(created_at), + "a note with no window is eligible in the block that created it", + ); + assert_eq!( + db.note_eligibility(windowed.as_note().id()).await, + Some(BlockNumber::from(100)), + "a note inside a future window waits for the window to open", + ); + assert_eq!( + db.note_eligibility(past_window.as_note().id()).await, + Some(created_at), + "a window that already opened does not move the note into the past", + ); +} + +#[tokio::test] +async fn failure_stores_the_backoff_derived_eligibility() { + let (db, _dir) = test_setup().await; + let account_id = mock_network_account_id(); + let note = mock_single_target_note(account_id, 1); + db.insert_network_notes(vec![note.clone()]).await.unwrap(); + + let failed_at = BlockNumber::from(50); + for attempt in 1..=3_usize { + db.notes_failed(vec![(note.as_note().nullifier(), test_note_error("boom"))], failed_at) + .await + .unwrap(); + + assert_eq!( + db.note_eligibility(note.as_note().id()).await, + Some(eligible_block_after_failure(note.execution_hint(), attempt, failed_at)), + "the stored block must match the backoff for the attempt count after the increment", + ); + } +} + +#[tokio::test] +async fn discard_pins_eligibility_beyond_every_block() { + let (db, _dir) = test_setup().await; + let account_id = mock_network_account_id(); + let note = mock_single_target_note(account_id, 1); + db.insert_network_notes(vec![note.clone()]).await.unwrap(); + + db.discard_notes(vec![note.as_note().nullifier()], BlockNumber::from(9), 30) + .await + .unwrap(); + + assert_eq!(db.note_eligibility(note.as_note().id()).await, Some(NEVER_ELIGIBLE)); +} + +/// A sponsorship arriving for a backed-off feature note makes the note eligible again. +#[tokio::test] +async fn arriving_sponsorship_clears_the_feature_note_backoff() { + let (db, _dir) = test_setup().await; + let account_id = mock_network_account_id(); + let feature = mock_single_target_note(account_id, 1); + db.insert_network_notes(vec![feature.clone()]).await.unwrap(); + + // The feature note failed, so it is waiting out a backoff. + db.notes_failed( + vec![(feature.as_note().nullifier(), test_note_error("fee not covered"))], + BlockNumber::from(10), + ) + .await + .unwrap(); + let backed_off = db.note_eligibility(feature.as_note().id()).await.unwrap(); + assert!(backed_off > BlockNumber::from(10)); + + let sponsorship_block = BlockNumber::from(11); + let effects = CommittedBlockEffects { + header: mock_block_header(sponsorship_block), + network_notes: vec![], + sponsorship_notes: vec![mock_sponsorship(account_id, feature.as_note().id(), 2)], + nullifiers: vec![], + network_account_updates: vec![], + account_transactions: vec![], + }; + db.apply_committed_block(effects, PartialMmr::default()).await.unwrap(); + + assert_eq!( + db.note_eligibility(feature.as_note().id()).await, + Some(sponsorship_block), + "the sponsorship is new information, so the note deserves an attempt now", + ); +} + +/// A sponsorship for a note that is already consumed changes nothing. +#[tokio::test] +async fn arriving_sponsorship_ignores_consumed_feature_notes() { + let (db, _dir) = test_setup().await; + let account_id = mock_network_account_id(); + let feature = mock_single_target_note(account_id, 1); + db.insert_network_notes(vec![feature.clone()]).await.unwrap(); + db.mark_notes_consumed(vec![feature.as_note().nullifier()], BlockNumber::from(5)) + .await + .unwrap(); + + let effects = CommittedBlockEffects { + header: mock_block_header(BlockNumber::from(6)), + network_notes: vec![], + sponsorship_notes: vec![mock_sponsorship(account_id, feature.as_note().id(), 2)], + nullifiers: vec![], + network_account_updates: vec![], + account_transactions: vec![], + }; + db.apply_committed_block(effects, PartialMmr::default()).await.unwrap(); + + assert_eq!( + db.note_eligibility(feature.as_note().id()).await, + Some(BlockNumber::GENESIS), + "a consumed note keeps the eligibility it was ingested with", + ); +} + // AVAILABLE NOTES + BACKOFF // ================================================================================================ diff --git a/bin/ntx-builder/src/test_utils.rs b/bin/ntx-builder/src/test_utils.rs index fcedb47526..96396be3a6 100644 --- a/bin/ntx-builder/src/test_utils.rs +++ b/bin/ntx-builder/src/test_utils.rs @@ -39,18 +39,36 @@ pub fn mock_single_target_note( mock_single_target_note_with_code(network_account_id, seed, None) } +/// Creates a `AccountTargetNetworkNote` carrying the given execution hint. +pub fn mock_single_target_note_with_hint( + network_account_id: AccountId, + seed: u8, + hint: NoteExecutionHint, +) -> AccountTargetNetworkNote { + mock_single_target_note_inner(network_account_id, seed, None, hint) +} + /// Creates a `AccountTargetNetworkNote` with optional custom note script code. pub fn mock_single_target_note_with_code( network_account_id: AccountId, seed: u8, code: Option<&str>, +) -> AccountTargetNetworkNote { + mock_single_target_note_inner(network_account_id, seed, code, NoteExecutionHint::Always) +} + +fn mock_single_target_note_inner( + network_account_id: AccountId, + seed: u8, + code: Option<&str>, + hint: NoteExecutionHint, ) -> AccountTargetNetworkNote { let mut rng = ChaCha20Rng::from_seed([seed; 32]); let sender = AccountIdBuilder::new() .account_type(AccountType::Private) .build_with_rng(&mut rng); - let target = NetworkAccountTarget::new(network_account_id, NoteExecutionHint::Always) + let target = NetworkAccountTarget::new(network_account_id, hint) .expect("network account should be valid target"); let mut builder = NoteBuilder::new(sender, rng).attachment(target); From ba85d25516180999f339db731245372553c5b3c4 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 10 Sep 2026 13:27:39 -0300 Subject: [PATCH 2/2] review: rneame tests, simplify comments, remove wrappers --- bin/ntx-builder/src/db/eligibility.rs | 119 ++---------------- .../src/db/queries/available_notes/mod.rs | 4 +- .../insert_network_note.sql | 11 +- .../db/queries/insert_network_notes/mod.rs | 8 +- .../src/db/queries/notes_failed/mod.rs | 28 ++--- .../select_note_attempt_state.sql | 2 + .../notes_failed/select_note_backoff.sql | 3 - .../reset_sponsored_notes.sql | 2 +- bin/ntx-builder/src/db/queries/tests.rs | 40 +++--- 9 files changed, 56 insertions(+), 161 deletions(-) create mode 100644 bin/ntx-builder/src/db/queries/notes_failed/select_note_attempt_state.sql delete mode 100644 bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql diff --git a/bin/ntx-builder/src/db/eligibility.rs b/bin/ntx-builder/src/db/eligibility.rs index 3032dfc35d..9def29553c 100644 --- a/bin/ntx-builder/src/db/eligibility.rs +++ b/bin/ntx-builder/src/db/eligibility.rs @@ -1,9 +1,9 @@ //! When a network note becomes eligible for a transaction attempt. //! -//! Two things delay a note: its execution hint, which opens a window of consumable blocks, and the -//! exponential backoff applied after a failed attempt. This module computes both, and every write -//! path that touches a note stores the result in `notes.next_eligible_block` so the scheduler can -//! ask for the ready accounts with a single indexed query. +//! Two things delay a note: its execution hint, which sets the first block at which the note may be +//! consumed, and the exponential backoff applied after a failed attempt. This module computes both, +//! and every write path that touches a note stores the result in `notes.next_eligible_block` so the +//! scheduler can ask for the ready accounts with a single indexed query. use miden_protocol::block::BlockNumber; use miden_standards::note::NoteExecutionHint; @@ -11,11 +11,6 @@ use miden_standards::note::NoteExecutionHint; /// Block number stored for a note that can never become eligible again. pub const NEVER_ELIGIBLE: BlockNumber = BlockNumber::MAX; -/// Returns the block at which a freshly ingested note becomes eligible. -pub fn first_eligible_block(hint: NoteExecutionHint, created_at: BlockNumber) -> BlockNumber { - eligible_at_or_after(hint, created_at) -} - /// Returns the block at which a note becomes eligible again after `attempts` failed attempts, the /// latest of which was recorded at `last_attempt`. pub fn eligible_block_after_failure( @@ -23,13 +18,7 @@ pub fn eligible_block_after_failure( attempts: usize, last_attempt: BlockNumber, ) -> BlockNumber { - eligible_at_or_after(hint, backoff_ready_block(Some(last_attempt), attempts)) -} - -/// Returns the first block at or after `floor` at which the note's execution hint permits -/// consumption. -fn eligible_at_or_after(hint: NoteExecutionHint, floor: BlockNumber) -> BlockNumber { - hint_next_consumable_block(hint, floor).map_or(floor, |block| block.max(floor)) + hint_floor(hint).max(backoff_ready_block(Some(last_attempt), attempts)) } /// Checks if the backoff block period has passed. @@ -79,44 +68,20 @@ pub fn note_recheck_block( if !backoff_ok { recheck = recheck.max(backoff_ready_block(last_attempt, attempts)); } - if !hint_ok && let Some(hint_block) = hint_next_consumable_block(hint, chain_tip) { - recheck = recheck.max(hint_block); + if !hint_ok { + recheck = recheck.max(hint_floor(hint)); } recheck } -/// Returns the first block at or after `from` for which `hint.can_be_consumed` turns true, or -/// `None` when the hint imposes no future-block constraint ([`NoteExecutionHint::None`], `Always` -/// or `Unknown`), leaving the caller's floor in place. -pub fn hint_next_consumable_block( - hint: NoteExecutionHint, - from: BlockNumber, -) -> Option { +/// Returns the first block at which the execution hint permits consumption. +pub fn hint_floor(hint: NoteExecutionHint) -> BlockNumber { match hint { - NoteExecutionHint::None | NoteExecutionHint::Always | NoteExecutionHint::Unknown(_) => None, - NoteExecutionHint::AfterBlock { block_num } => Some(block_num), - NoteExecutionHint::OnBlockSlot { round_len, slot_len, slot_offset } => { - let block = u64::from(from.as_u32()); - // `1 << round_len` as `can_be_consumed` computes it, in u64 to avoid the overflow its - // u32 shift would hit; bail to the caller's floor for degenerate exponents. - let round_len_blocks = 1u64.checked_shl(u32::from(round_len))?; - let slot_len_blocks = 1u64.checked_shl(u32::from(slot_len))?; - let round_index = block / round_len_blocks; - let slot_start = - round_index * round_len_blocks + u64::from(slot_offset) * slot_len_blocks; - let slot_end = slot_start + slot_len_blocks; - let next = if block < slot_start { - slot_start - } else if block >= slot_end { - // Past this round's slot; the next opening is the same slot one round later. - slot_start + round_len_blocks - } else { - block - }; - // Beyond the representable block range the note is effectively never consumable; clamp - // so the caller schedules at most a far-future recheck rather than wrapping. - Some(BlockNumber::from(u32::try_from(next).unwrap_or(u32::MAX))) + NoteExecutionHint::None | NoteExecutionHint::Always | NoteExecutionHint::Unknown(_) => { + BlockNumber::GENESIS }, + NoteExecutionHint::AfterBlock { block_num } => block_num, + NoteExecutionHint::OnBlockSlot { .. } => NEVER_ELIGIBLE, } } @@ -124,40 +89,6 @@ pub fn hint_next_consumable_block( mod tests { use super::*; - /// Brute-forces the first block at or after `from` for which the hint is consumable, by - /// scanning forward. Used as an independent oracle for [`hint_next_consumable_block`]. - fn brute_force_next(hint: NoteExecutionHint, from: u32) -> Option { - (from..=from.saturating_add(4096)) - .find(|&b| hint.can_be_consumed(BlockNumber::from(b)) == Some(true)) - } - - /// [`hint_next_consumable_block`] must agree, block for block, with scanning - /// [`NoteExecutionHint::can_be_consumed`] forward. This guards against the slot arithmetic - /// drifting from the protocol definition it mirrors. - #[test] - fn hint_next_consumable_block_matches_can_be_consumed() { - let hints = [ - NoteExecutionHint::after_block(BlockNumber::from(200)), - NoteExecutionHint::on_block_slot(10, 7, 1), - NoteExecutionHint::on_block_slot(8, 4, 0), - NoteExecutionHint::on_block_slot(9, 5, 3), - ]; - for hint in hints { - for b in 0u32..1300 { - // Only meaningful while the note is currently NOT consumable. - if hint.can_be_consumed(BlockNumber::from(b)) != Some(false) { - continue; - } - let got = hint_next_consumable_block(hint, BlockNumber::from(b)) - .expect("a windowed hint must report a next block") - .as_u32(); - let expected = brute_force_next(hint, b) - .expect("oracle must find a consumable block within the scan window"); - assert_eq!(got, expected, "hint {hint:?} at block {b}"); - } - } - } - #[rstest::rstest] #[test] #[case::all_zero(Some(BlockNumber::GENESIS), BlockNumber::GENESIS, 0, true)] @@ -224,28 +155,4 @@ mod tests { backoff_ready_block(Some(last_attempt), 1), ); } - - /// An ingested note is eligible immediately unless its hint says otherwise. - #[test] - fn first_eligible_block_follows_the_hint() { - let created_at = BlockNumber::from(42); - - assert_eq!( - first_eligible_block(NoteExecutionHint::Always, created_at), - created_at, - "an unconstrained note is eligible in the block that created it", - ); - assert_eq!( - first_eligible_block( - NoteExecutionHint::after_block(BlockNumber::from(100)), - created_at - ), - BlockNumber::from(100), - ); - assert_eq!( - first_eligible_block(NoteExecutionHint::after_block(BlockNumber::from(7)), created_at), - created_at, - "a window that already opened does not move the note into the past", - ); - } } diff --git a/bin/ntx-builder/src/db/queries/available_notes/mod.rs b/bin/ntx-builder/src/db/queries/available_notes/mod.rs index 3bdb570c74..dc1c7bfbdb 100644 --- a/bin/ntx-builder/src/db/queries/available_notes/mod.rs +++ b/bin/ntx-builder/src/db/queries/available_notes/mod.rs @@ -7,7 +7,7 @@ use miden_protocol::block::BlockNumber; use miden_protocol::note::Note; use miden_standards::note::AccountTargetNetworkNote; -use crate::db::eligibility::{has_backoff_passed, note_recheck_block}; +use crate::db::eligibility::{has_backoff_passed, hint_floor, note_recheck_block}; const SQL: &str = include_str!("available_notes.sql"); @@ -53,7 +53,7 @@ pub fn available_notes( })?; let hint = note.execution_hint(); - let hint_ok = hint.can_be_consumed(block_num).unwrap_or(true); + let hint_ok = block_num >= hint_floor(hint); let backoff_ok = has_backoff_passed(block_num, last_attempt, attempt_count); if hint_ok && backoff_ok { eligible.push(note); diff --git a/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql b/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql index 0efdc48581..be5e815374 100644 --- a/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql +++ b/bin/ntx-builder/src/db/queries/insert_network_notes/insert_network_note.sql @@ -1,9 +1,4 @@ --- Inserts a network note from a committed block. Uses `INSERT OR IGNORE` so re-applying the same --- block (e.g. on a redelivery from the subscription stream) is a no-op rather than a constraint --- violation. `attempt_count` defaults to 0 and the remaining backoff/lifecycle columns default to --- NULL. --- --- `next_eligible_block` is the first block at which the note's execution hint permits consumption. --- A note that has never been attempted has no backoff, so the hint is the only delay. -INSERT OR IGNORE INTO notes (nullifier, account_id, note_data, note_id, next_eligible_block) +-- Inserts a network note. `attempt_count` defaults to 0 and the remaining backoff/lifecycle +-- columns default to NULL. +INSERT INTO notes (nullifier, account_id, note_data, note_id, next_eligible_block) VALUES (?1, ?2, ?3, ?4, ?5) diff --git a/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs b/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs index 4856b8eb62..54081d9ee1 100644 --- a/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs +++ b/bin/ntx-builder/src/db/queries/insert_network_notes/mod.rs @@ -5,13 +5,11 @@ use miden_node_db::{DatabaseError, SqlTypeConvert}; use miden_protocol::block::BlockNumber; use miden_standards::note::AccountTargetNetworkNote; -use crate::db::eligibility::first_eligible_block; +use crate::db::eligibility::hint_floor; const SQL: &str = include_str!("insert_network_note.sql"); -/// Inserts network notes created by the block at `created_at`. Uses `INSERT OR IGNORE` so -/// re-applying the same block (e.g. on a redelivery from the subscription stream) is a no-op rather -/// than a constraint violation. +/// Inserts network notes created by the block at `created_at`. /// /// Each note's `next_eligible_block` is derived from its execution hint, so a note inside a future /// window is not selected before the window opens. @@ -22,7 +20,7 @@ pub fn insert_network_notes( ) -> Result<(), DatabaseError> { for note in notes { let inner = note.as_note(); - let eligible_from = first_eligible_block(note.execution_hint(), created_at); + let eligible_from = hint_floor(note.execution_hint()).max(created_at); tx.execute( SQL, &[ diff --git a/bin/ntx-builder/src/db/queries/notes_failed/mod.rs b/bin/ntx-builder/src/db/queries/notes_failed/mod.rs index 5fd78f9d50..cb742fa4e2 100644 --- a/bin/ntx-builder/src/db/queries/notes_failed/mod.rs +++ b/bin/ntx-builder/src/db/queries/notes_failed/mod.rs @@ -11,7 +11,7 @@ use crate::NoteError; use crate::db::eligibility::eligible_block_after_failure; const SQL: &str = include_str!("note_failed.sql"); -const SELECT_BACKOFF_SQL: &str = include_str!("select_note_backoff.sql"); +const SELECT_ATTEMPT_STATE_SQL: &str = include_str!("select_note_attempt_state.sql"); /// Marks notes as failed by incrementing `attempt_count`, setting `last_attempt`, storing the /// latest error message, and moving `next_eligible_block` to the end of the new backoff window. @@ -23,41 +23,35 @@ pub fn notes_failed( let block_num_val = block_num.to_raw_sql(); for (nullifier, error) in failed_notes { - let Some(eligible_from) = next_eligible_block(tx, nullifier, block_num)? else { - // The note is gone, so there is nothing to penalize. - continue; - }; + let eligible_from = eligibility_after_failure(tx, nullifier, block_num)?; let error_report = error.as_report(); tx.execute(SQL, &[nullifier, &block_num_val, &error_report, &eligible_from.to_raw_sql()])?; } Ok(()) } -/// Returns the eligibility block to store for a note that is failing now, or `None` when the note -/// has no row. -fn next_eligible_block( +/// Returns the value to store in `notes.next_eligible_block` for a note that is failing now. +fn eligibility_after_failure( tx: &WriteTx<'_>, nullifier: &Nullifier, block_num: BlockNumber, -) -> Result, DatabaseError> { +) -> Result { #[expect(clippy::cast_sign_loss)] - let row = tx - .query(SELECT_BACKOFF_SQL, &[nullifier], |row| { + let (attempt_count, note) = tx + .query(SELECT_ATTEMPT_STATE_SQL, &[nullifier], |row| { Ok((row.get::(0)? as usize, row.get::(1)?)) })? .into_iter() - .next(); + .next() + .expect("a failed note must have a row"); - let Some((attempt_count, note)) = row else { - return Ok(None); - }; let note = AccountTargetNetworkNote::new(note).map_err(|source| { DatabaseError::deserialization("failed to convert to network note", source) })?; - Ok(Some(eligible_block_after_failure( + Ok(eligible_block_after_failure( note.execution_hint(), attempt_count + 1, block_num, - ))) + )) } diff --git a/bin/ntx-builder/src/db/queries/notes_failed/select_note_attempt_state.sql b/bin/ntx-builder/src/db/queries/notes_failed/select_note_attempt_state.sql new file mode 100644 index 0000000000..430bf052c3 --- /dev/null +++ b/bin/ntx-builder/src/db/queries/notes_failed/select_note_attempt_state.sql @@ -0,0 +1,2 @@ +-- Returns the attempt count and the serialized note for one note. +SELECT attempt_count, note_data FROM notes WHERE nullifier = ?1 diff --git a/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql b/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql deleted file mode 100644 index 97e0998ac5..0000000000 --- a/bin/ntx-builder/src/db/queries/notes_failed/select_note_backoff.sql +++ /dev/null @@ -1,3 +0,0 @@ --- Reads what the new eligibility block of a failing note depends on: its current attempt count and --- its execution hint (carried by the serialized note). -SELECT attempt_count, note_data FROM notes WHERE nullifier = ?1 diff --git a/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql index 2ea507f5b0..d5d293be6f 100644 --- a/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql +++ b/bin/ntx-builder/src/db/queries/reset_sponsored_notes/reset_sponsored_notes.sql @@ -1,4 +1,4 @@ --- Makes the pending feature notes that just gained a `FEE_SPONSORSHIP` note eligible again. +-- Sets `next_eligible_block` for every unconsumed note in the given note id list. UPDATE notes SET next_eligible_block = ?2 diff --git a/bin/ntx-builder/src/db/queries/tests.rs b/bin/ntx-builder/src/db/queries/tests.rs index 371cb3733c..4ec4c7c837 100644 --- a/bin/ntx-builder/src/db/queries/tests.rs +++ b/bin/ntx-builder/src/db/queries/tests.rs @@ -52,19 +52,6 @@ async fn upsert_account_replaces_existing_row() { // NETWORK NOTE INSERT/DELETE // ================================================================================================ -#[tokio::test] -async fn insert_network_notes_is_idempotent() { - let (db, _dir) = test_setup().await; - let account_id = mock_network_account_id(); - let note = mock_single_target_note(account_id, 7); - - db.insert_network_notes(vec![note.clone()]).await.unwrap(); - // Re-applying the same block (e.g. on a subscription redelivery) must not error or duplicate. - db.insert_network_notes(vec![note]).await.unwrap(); - - assert_eq!(db.count_notes().await, 1); -} - #[tokio::test] async fn mark_notes_consumed_keeps_rows_and_sets_committed_at() { let (db, _dir) = test_setup().await; @@ -297,7 +284,7 @@ async fn apply_committed_block_returns_sponsored_account_wakeups() { // the read-time check. #[tokio::test] -async fn ingestion_stores_the_hint_derived_eligibility() { +async fn insert_network_notes_stores_hint_derived_eligibility() { let (db, _dir) = test_setup().await; let account_id = mock_network_account_id(); let created_at = BlockNumber::from(40); @@ -313,9 +300,19 @@ async fn ingestion_stores_the_hint_derived_eligibility() { 3, NoteExecutionHint::after_block(BlockNumber::from(7)), ); + let slot_windowed = mock_single_target_note_with_hint( + account_id, + 4, + NoteExecutionHint::on_block_slot(10, 7, 1), + ); db.insert_network_notes_at( - vec![unconstrained.clone(), windowed.clone(), past_window.clone()], + vec![ + unconstrained.clone(), + windowed.clone(), + past_window.clone(), + slot_windowed.clone(), + ], created_at, ) .await @@ -336,10 +333,15 @@ async fn ingestion_stores_the_hint_derived_eligibility() { Some(created_at), "a window that already opened does not move the note into the past", ); + assert_eq!( + db.note_eligibility(slot_windowed.as_note().id()).await, + Some(NEVER_ELIGIBLE), + "a slot-windowed note is never eligible", + ); } #[tokio::test] -async fn failure_stores_the_backoff_derived_eligibility() { +async fn notes_failed_stores_backoff_derived_eligibility() { let (db, _dir) = test_setup().await; let account_id = mock_network_account_id(); let note = mock_single_target_note(account_id, 1); @@ -360,7 +362,7 @@ async fn failure_stores_the_backoff_derived_eligibility() { } #[tokio::test] -async fn discard_pins_eligibility_beyond_every_block() { +async fn discard_notes_pins_eligibility_to_never() { let (db, _dir) = test_setup().await; let account_id = mock_network_account_id(); let note = mock_single_target_note(account_id, 1); @@ -375,7 +377,7 @@ async fn discard_pins_eligibility_beyond_every_block() { /// A sponsorship arriving for a backed-off feature note makes the note eligible again. #[tokio::test] -async fn arriving_sponsorship_clears_the_feature_note_backoff() { +async fn reset_sponsored_notes_clears_feature_note_backoff() { let (db, _dir) = test_setup().await; let account_id = mock_network_account_id(); let feature = mock_single_target_note(account_id, 1); @@ -411,7 +413,7 @@ async fn arriving_sponsorship_clears_the_feature_note_backoff() { /// A sponsorship for a note that is already consumed changes nothing. #[tokio::test] -async fn arriving_sponsorship_ignores_consumed_feature_notes() { +async fn reset_sponsored_notes_skips_consumed_feature_notes() { let (db, _dir) = test_setup().await; let account_id = mock_network_account_id(); let feature = mock_single_target_note(account_id, 1);