Skip to content

Token: transfer_from and burn_from extend a temporary entry past the network TTL ceiling, so partial spends revert (regression of #344) #399

Description

@zachyo

Files: contracts/token/src/lib.rs lines 594 to 599 (transfer_from) and 644 to 649 (burn_from); compare line 558 (approve)

Issue: #344 added the clamp to approve and to nowhere else. approve is correct:

// lib.rs:558
let ttl_ledgers = (expiration_ledger - current_ledger).min(env.storage().max_ttl());

transfer_from and burn_from, which rewrite the same entry when an allowance is partially spent, are not:

// lib.rs:594 (transfer_from) and 644 (burn_from), identical
let ttl_ledgers = expiration_ledger.saturating_sub(current_ledger);
if ttl_ledgers > 0 {
    env.storage().temporary().extend_ttl(&key, ttl_ledgers, ttl_ledgers);
}

Allowances are temporary entries since #326, and temporary is the durability for which the host refuses to clamp (soroban-env-host-21.2.1/src/storage.rs:527-535, "for Temporary entries TTL has to be exact"). It returns Storage/InvalidAction: trying to extend past max live_until ledger.

So for any allowance whose expiration_ledger is more than max_entry_ttl (3,110,400) ledgers out:

  • approve succeeds, because it clamps the TTL. Note it stores the unclamped expiration_ledger in the AllowanceValue struct, which is what the two functions above then read back.
  • A partial transfer_from or burn_from reverts.
  • A full spend succeeds, because remaining == 0 takes the remove branch at line 601 and never calls extend_ttl.

A spender who is approved for 1,000 and takes 400 fails; the same spender taking all 1,000 succeeds. That reads as an intermittent, amount-dependent failure with an opaque host error, which is close to undiagnosable from the UI, and #347 means the error string is stripped in release anyway.

This is reachable through the app's default path, not an edge case: ApproveForm defaults to a 365-day expiry, which is 6,307,200 ledgers. See #400.

Fix: Apply .min(env.storage().max_ttl()) at both sites. Better, clamp expiration_ledger itself in approve before storing it, so the struct never carries a value the network cannot honour and all three call sites agree by construction. Add a test that approves past the maximum and asserts a partial transfer_from succeeds; set the test ledger's max_entry_ttl to the real 3,110,400 with env.ledger().set_max_entry_ttl(...), because the harness default of 6,312,000 hides this entirely.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions