From dede80d117fbb12ef589a268745fbda8cbbcff05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Akdo=C4=9Fan?= <70746444+furkan3152@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:00:55 +0000 Subject: [PATCH 1/4] fix(confidential): reject expired live_until_ledger in set_spender set_spender accepted a live_until_ledger lower than the current ledger, creating a delegation that is expired on arrival while still escrowing the allowance. Reject it with ConfidentialTokenError::InvalidLiveUntilLedger (3515) before the auditor lookup and proof verification. The bound is inclusive: live_until_ledger equal to the current ledger is accepted. Fixes #947 --- packages/confidential/src/lib.rs | 8 ++++++-- packages/confidential/src/storage.rs | 9 +++++++++ packages/confidential/src/test.rs | 30 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/confidential/src/lib.rs b/packages/confidential/src/lib.rs index 42b8b367a..4bb3fda95 100644 --- a/packages/confidential/src/lib.rs +++ b/packages/confidential/src/lib.rs @@ -441,8 +441,8 @@ pub trait ConfidentialToken { /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation /// expires. Spending is authorized while `ledger.sequence() <= - /// live_until_ledger`. The escrowed value persists until - /// [`ConfidentialToken::revoke_spender`]. + /// live_until_ledger`. Must not be lower than the current ledger. The + /// escrowed value persists until [`ConfidentialToken::revoke_spender`]. /// * `data` - XDR-encoded [`SetSpenderData`]. /// /// # Errors @@ -596,6 +596,10 @@ pub enum ConfidentialTokenError { /// boundary to keep stored state and emitted events byte-unique per /// logical value. NonCanonicalEncoding = 3514, + /// Indicates `live_until_ledger` is already in the past + /// (`live_until_ledger < ledger.sequence()`), so the delegation would be + /// expired on creation. + InvalidLiveUntilLedger = 3515, } // ################## CONSTANTS ################## diff --git a/packages/confidential/src/storage.rs b/packages/confidential/src/storage.rs index 2f894263a..f66bb6a8f 100644 --- a/packages/confidential/src/storage.rs +++ b/packages/confidential/src/storage.rs @@ -841,11 +841,14 @@ pub fn confidential_transfer_from( /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation expires. /// Spending is authorized while `ledger.sequence() <= live_until_ledger`. +/// Must not be lower than the current ledger. /// * `payload` - The decoded [`SetSpenderPayload`]. /// * `proof` - The raw UltraHonk proof bytes. /// /// # Errors /// +/// * [`ConfidentialTokenError::InvalidLiveUntilLedger`] - When +/// `live_until_ledger` is lower than the current ledger sequence. /// * [`ConfidentialTokenError::AccountNotRegistered`] - When `account` or /// `spender` is not registered. /// * [`ConfidentialTokenError::DelegationAlreadyExists`] - When a delegation @@ -874,6 +877,12 @@ pub fn set_spender( payload: &SetSpenderPayload, proof: &Bytes, ) { + // Reject a delegation that would already be expired before doing any + // external call or proof verification. + if live_until_ledger < e.ledger().sequence() { + panic_with_error!(e, ConfidentialTokenError::InvalidLiveUntilLedger); + } + let owner = get_account(e, account); let spender_account = get_account(e, spender); let auditor = ConfidentialAuditorClient::new(e, &get_auditor(e)); diff --git a/packages/confidential/src/test.rs b/packages/confidential/src/test.rs index e5dae3de2..3bf0843a0 100644 --- a/packages/confidential/src/test.rs +++ b/packages/confidential/src/test.rs @@ -713,6 +713,36 @@ fn set_spender_twice_panics() { h.token.set_spender(&alice, &spender, &1_000u32, &set_spender_data(&h.e)); } +#[test] +#[should_panic(expected = "Error(Contract, #3515)")] +fn set_spender_with_past_live_until_ledger_panics() { + let h = setup(); + let alice = Address::generate(&h.e); + let spender = Address::generate(&h.e); + + h.token.register(&alice, &1u32, ®ister_data(&h.e)); + h.token.register(&spender, &1u32, ®ister_data(&h.e)); + + h.e.ledger().set_sequence_number(100); + h.token.set_spender(&alice, &spender, &99u32, &set_spender_data(&h.e)); +} + +#[test] +fn set_spender_with_current_ledger_as_live_until_ledger_succeeds() { + let h = setup(); + let alice = Address::generate(&h.e); + let spender = Address::generate(&h.e); + + h.token.register(&alice, &1u32, ®ister_data(&h.e)); + h.token.register(&spender, &1u32, ®ister_data(&h.e)); + + h.e.ledger().set_sequence_number(100); + h.token.set_spender(&alice, &spender, &100u32, &set_spender_data(&h.e)); + + assert_eq!(h.token.get_spender_delegation(&alice, &spender).live_until_ledger, 100); + assert!(h.token.is_spender(&alice, &spender)); +} + #[test] fn revoke_spender_deletes_delegation() { let h = setup(); From fea48991ae32cba8fc420181be01ae51825aa2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Akdo=C4=9Fan?= <70746444+furkan3152@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:04:00 +0000 Subject: [PATCH 2/4] docs(confidential): cite the live_until_ledger rule from the spec Add the set_spender lower bound to account-state.md and cite it from the set_spender docs. Fix the ConfidentialTokenError range in the package guide to match Architecture.md. --- packages/confidential/CLAUDE.md | 2 +- packages/confidential/docs/protocol/account-state.md | 2 +- packages/confidential/src/lib.rs | 5 +++-- packages/confidential/src/storage.rs | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/confidential/CLAUDE.md b/packages/confidential/CLAUDE.md index adebe4324..6ee1ce871 100644 --- a/packages/confidential/CLAUDE.md +++ b/packages/confidential/CLAUDE.md @@ -19,7 +19,7 @@ Balances are Pedersen commitments on Grumpkin. Every operation that opens or re- ## Errors -`ConfidentialTokenError` occupies **3500–3510**. The root guide's range list predates this module and omits it. Stay in the 3500s; do not open a new range for the satellites. +`ConfidentialTokenError` occupies **3500–3599**. The root guide's range list predates this module and omits it. Stay in the 3500s; do not open a new range for the satellites. ## Canonical encoding is a security boundary diff --git a/packages/confidential/docs/protocol/account-state.md b/packages/confidential/docs/protocol/account-state.md index 1bd01f2e1..9057f979f 100644 --- a/packages/confidential/docs/protocol/account-state.md +++ b/packages/confidential/docs/protocol/account-state.md @@ -72,7 +72,7 @@ A spender transfer avoids this by splitting the salt's two jobs: opening the sto ### `live_until_ledger` -The ledger number at which the delegation expires. The delegation is live while `ledger.sequence() <= live_until_ledger` and expired once `ledger.sequence() > live_until_ledger`. Checked on every `confidential_transfer_from`. The entry lives in persistent, not temporary, storage: automatic cleanup would destroy escrowed funds. +The ledger number at which the delegation expires. The delegation is live while `ledger.sequence() <= live_until_ledger` and expired once `ledger.sequence() > live_until_ledger`. Checked on every `confidential_transfer_from`. `set_spender` rejects a `live_until_ledger` below the current ledger, so a delegation is never created already expired. The entry lives in persistent, not temporary, storage: automatic cleanup would destroy escrowed funds. The `(owner, spender)` storage entry holds at most one delegation. `set_spender` ([Set Spender](operations/set-spender.md)) reverts if a delegation already exists for that pair, regardless of whether the existing delegation is past `live_until_ledger`. Expiry only prevents the spender from spending; the escrowed value persists on-chain until `revoke_spender` ([Revoke Spender](operations/revoke-spender.md)) folds it back into the owner's spendable balance. Re-delegating to the same spender therefore requires the sequence: `revoke_spender` then `set_spender`. This rule is what keeps the balance-conservation invariant ([Balance Conservation](security.md#balance-conservation)) well-defined over stored delegations: every delegation is either active, expired-pending-revoke, or absent, and the escrowed value is never silently dropped. diff --git a/packages/confidential/src/lib.rs b/packages/confidential/src/lib.rs index 4bb3fda95..d778fb25f 100644 --- a/packages/confidential/src/lib.rs +++ b/packages/confidential/src/lib.rs @@ -441,8 +441,9 @@ pub trait ConfidentialToken { /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation /// expires. Spending is authorized while `ledger.sequence() <= - /// live_until_ledger`. Must not be lower than the current ledger. The - /// escrowed value persists until [`ConfidentialToken::revoke_spender`]. + /// live_until_ledger`. Must not be lower than the current ledger + /// (`docs/protocol/account-state.md#live_until_ledger`). The escrowed + /// value persists until [`ConfidentialToken::revoke_spender`]. /// * `data` - XDR-encoded [`SetSpenderData`]. /// /// # Errors diff --git a/packages/confidential/src/storage.rs b/packages/confidential/src/storage.rs index f66bb6a8f..982962d8c 100644 --- a/packages/confidential/src/storage.rs +++ b/packages/confidential/src/storage.rs @@ -841,7 +841,8 @@ pub fn confidential_transfer_from( /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation expires. /// Spending is authorized while `ledger.sequence() <= live_until_ledger`. -/// Must not be lower than the current ledger. +/// Must not be lower than the current ledger +/// (`docs/protocol/account-state.md#live_until_ledger`). /// * `payload` - The decoded [`SetSpenderPayload`]. /// * `proof` - The raw UltraHonk proof bytes. /// From ed81f7053aa31963e1f99835db6467066f443ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Akdo=C4=9Fan?= <70746444+furkan3152@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:43:12 +0000 Subject: [PATCH 3/4] fix(confidential): also bound live_until_ledger by max_live_until_ledger Match the fungible set_allowance and reject a live_until_ledger above max_live_until_ledger. Drop the spec and citation changes from the previous commit, as asked in review. --- packages/confidential/CLAUDE.md | 2 +- .../docs/protocol/account-state.md | 2 +- packages/confidential/src/lib.rs | 12 ++++---- packages/confidential/src/storage.rs | 14 +++++---- packages/confidential/src/test.rs | 29 +++++++++++++++++++ 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/packages/confidential/CLAUDE.md b/packages/confidential/CLAUDE.md index 6ee1ce871..adebe4324 100644 --- a/packages/confidential/CLAUDE.md +++ b/packages/confidential/CLAUDE.md @@ -19,7 +19,7 @@ Balances are Pedersen commitments on Grumpkin. Every operation that opens or re- ## Errors -`ConfidentialTokenError` occupies **3500–3599**. The root guide's range list predates this module and omits it. Stay in the 3500s; do not open a new range for the satellites. +`ConfidentialTokenError` occupies **3500–3510**. The root guide's range list predates this module and omits it. Stay in the 3500s; do not open a new range for the satellites. ## Canonical encoding is a security boundary diff --git a/packages/confidential/docs/protocol/account-state.md b/packages/confidential/docs/protocol/account-state.md index 9057f979f..1bd01f2e1 100644 --- a/packages/confidential/docs/protocol/account-state.md +++ b/packages/confidential/docs/protocol/account-state.md @@ -72,7 +72,7 @@ A spender transfer avoids this by splitting the salt's two jobs: opening the sto ### `live_until_ledger` -The ledger number at which the delegation expires. The delegation is live while `ledger.sequence() <= live_until_ledger` and expired once `ledger.sequence() > live_until_ledger`. Checked on every `confidential_transfer_from`. `set_spender` rejects a `live_until_ledger` below the current ledger, so a delegation is never created already expired. The entry lives in persistent, not temporary, storage: automatic cleanup would destroy escrowed funds. +The ledger number at which the delegation expires. The delegation is live while `ledger.sequence() <= live_until_ledger` and expired once `ledger.sequence() > live_until_ledger`. Checked on every `confidential_transfer_from`. The entry lives in persistent, not temporary, storage: automatic cleanup would destroy escrowed funds. The `(owner, spender)` storage entry holds at most one delegation. `set_spender` ([Set Spender](operations/set-spender.md)) reverts if a delegation already exists for that pair, regardless of whether the existing delegation is past `live_until_ledger`. Expiry only prevents the spender from spending; the escrowed value persists on-chain until `revoke_spender` ([Revoke Spender](operations/revoke-spender.md)) folds it back into the owner's spendable balance. Re-delegating to the same spender therefore requires the sequence: `revoke_spender` then `set_spender`. This rule is what keeps the balance-conservation invariant ([Balance Conservation](security.md#balance-conservation)) well-defined over stored delegations: every delegation is either active, expired-pending-revoke, or absent, and the escrowed value is never silently dropped. diff --git a/packages/confidential/src/lib.rs b/packages/confidential/src/lib.rs index d778fb25f..1d5f0032e 100644 --- a/packages/confidential/src/lib.rs +++ b/packages/confidential/src/lib.rs @@ -441,9 +441,9 @@ pub trait ConfidentialToken { /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation /// expires. Spending is authorized while `ledger.sequence() <= - /// live_until_ledger`. Must not be lower than the current ledger - /// (`docs/protocol/account-state.md#live_until_ledger`). The escrowed - /// value persists until [`ConfidentialToken::revoke_spender`]. + /// live_until_ledger`. Must be between the current ledger and + /// `max_live_until_ledger`, inclusive. The escrowed value persists until + /// [`ConfidentialToken::revoke_spender`]. /// * `data` - XDR-encoded [`SetSpenderData`]. /// /// # Errors @@ -597,9 +597,9 @@ pub enum ConfidentialTokenError { /// boundary to keep stored state and emitted events byte-unique per /// logical value. NonCanonicalEncoding = 3514, - /// Indicates `live_until_ledger` is already in the past - /// (`live_until_ledger < ledger.sequence()`), so the delegation would be - /// expired on creation. + /// Indicates `live_until_ledger` is outside the accepted range: below the + /// current ledger, so the delegation would be expired on creation, or above + /// `max_live_until_ledger`. InvalidLiveUntilLedger = 3515, } diff --git a/packages/confidential/src/storage.rs b/packages/confidential/src/storage.rs index 982962d8c..c91ce581d 100644 --- a/packages/confidential/src/storage.rs +++ b/packages/confidential/src/storage.rs @@ -841,15 +841,15 @@ pub fn confidential_transfer_from( /// account so its spending key is available for the `dvk` escrow ECDH. /// * `live_until_ledger` - The ledger number at which the delegation expires. /// Spending is authorized while `ledger.sequence() <= live_until_ledger`. -/// Must not be lower than the current ledger -/// (`docs/protocol/account-state.md#live_until_ledger`). +/// Must be between the current ledger and `max_live_until_ledger`, inclusive. /// * `payload` - The decoded [`SetSpenderPayload`]. /// * `proof` - The raw UltraHonk proof bytes. /// /// # Errors /// /// * [`ConfidentialTokenError::InvalidLiveUntilLedger`] - When -/// `live_until_ledger` is lower than the current ledger sequence. +/// `live_until_ledger` is lower than the current ledger sequence or greater +/// than `max_live_until_ledger`. /// * [`ConfidentialTokenError::AccountNotRegistered`] - When `account` or /// `spender` is not registered. /// * [`ConfidentialTokenError::DelegationAlreadyExists`] - When a delegation @@ -878,9 +878,11 @@ pub fn set_spender( payload: &SetSpenderPayload, proof: &Bytes, ) { - // Reject a delegation that would already be expired before doing any - // external call or proof verification. - if live_until_ledger < e.ledger().sequence() { + // Same bounds as the fungible `set_allowance`, checked before any external + // call or proof verification. + if live_until_ledger < e.ledger().sequence() + || live_until_ledger > e.ledger().max_live_until_ledger() + { panic_with_error!(e, ConfidentialTokenError::InvalidLiveUntilLedger); } diff --git a/packages/confidential/src/test.rs b/packages/confidential/src/test.rs index 3bf0843a0..cc060d212 100644 --- a/packages/confidential/src/test.rs +++ b/packages/confidential/src/test.rs @@ -727,6 +727,35 @@ fn set_spender_with_past_live_until_ledger_panics() { h.token.set_spender(&alice, &spender, &99u32, &set_spender_data(&h.e)); } +#[test] +#[should_panic(expected = "Error(Contract, #3515)")] +fn set_spender_beyond_max_live_until_ledger_panics() { + let h = setup(); + let alice = Address::generate(&h.e); + let spender = Address::generate(&h.e); + + h.token.register(&alice, &1u32, ®ister_data(&h.e)); + h.token.register(&spender, &1u32, ®ister_data(&h.e)); + + let max = h.e.ledger().max_live_until_ledger(); + h.token.set_spender(&alice, &spender, &(max + 1), &set_spender_data(&h.e)); +} + +#[test] +fn set_spender_with_max_live_until_ledger_succeeds() { + let h = setup(); + let alice = Address::generate(&h.e); + let spender = Address::generate(&h.e); + + h.token.register(&alice, &1u32, ®ister_data(&h.e)); + h.token.register(&spender, &1u32, ®ister_data(&h.e)); + + let max = h.e.ledger().max_live_until_ledger(); + h.token.set_spender(&alice, &spender, &max, &set_spender_data(&h.e)); + + assert_eq!(h.token.get_spender_delegation(&alice, &spender).live_until_ledger, max); +} + #[test] fn set_spender_with_current_ledger_as_live_until_ledger_succeeds() { let h = setup(); From 381c8e542ce769cf63dcf40e9c3b5e9707ab2916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Akdo=C4=9Fan?= <70746444+furkan3152@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:04:50 +0000 Subject: [PATCH 4/4] docs(confidential): state where the live_until_ledger check runs --- packages/confidential/src/storage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/confidential/src/storage.rs b/packages/confidential/src/storage.rs index c91ce581d..86fc1148f 100644 --- a/packages/confidential/src/storage.rs +++ b/packages/confidential/src/storage.rs @@ -878,8 +878,8 @@ pub fn set_spender( payload: &SetSpenderPayload, proof: &Bytes, ) { - // Same bounds as the fungible `set_allowance`, checked before any external - // call or proof verification. + // Same bounds as the fungible `set_allowance`, checked before the account + // lookups, the auditor call and proof verification. if live_until_ledger < e.ledger().sequence() || live_until_ledger > e.ledger().max_live_until_ledger() {