Skip to content

[#546] Define offer expiration at exact ledger boundaries - #568

Open
woahwhattheheck wants to merge 1 commit into
Commitlabs-Org:masterfrom
woahwhattheheck:swe12/offer-expiry-boundaries-546-20261008
Open

woahwhattheheck wants to merge 1 commit into
Commitlabs-Org:masterfrom
woahwhattheheck:swe12/offer-expiry-boundaries-546-20261008

Conversation

@woahwhattheheck

Copy link
Copy Markdown

Summary

Fixes #546 — offers previously had no expiration at all: Offer carried
created_at only, and accept_offer could settle a stale offer at any
future ledger time. This PR adds an explicit, documented expiry policy.

  • Offer gains expires_at: u64, computed at creation as
    created_at + duration using checked arithmetic.
  • make_offer takes a new duration: u64 parameter (ledger seconds).
  • A single predicate, is_offer_expired(now, expires_at), centralizes all
    offer-expiry comparisons.
  • The OfferMade event payload now exposes the effective expires_at.
  • contracts/commitment_marketplace/README.md and
    docs/commitment_marketplace.md document the policy and signature.

Expiration policy (documented)

The boundary is exclusive — the same convention Auction::ends_at
already uses in this contract: an offer is valid only while
ledger.timestamp() < expires_at. At the exact expiry instant the offer is
already expired.

  • Creation: duration == 0 or an overflowing created_at + duration
    fails with InvalidDuration (mirrors start_auction).
  • Update: offers are immutable while live (a duplicate make_offer
    fails with OfferExists). Once expired, a fresh make_offer by the same
    offerer replaces the stale offer in place — the update path.
  • Acceptance: accept_offer rejects offers at or past expires_at
    with the new OfferExpired error (feat: implement comprehensive compliance verification for commitments #35, appended; existing discriminants
    unchanged) before any token movement, and does not mutate the offer.
  • Cancellation: cancel_offer is allowed before, at, or after expiry —
    it is also the manual cleanup path for expired offers.

Acceptance criteria

  • An offer at the exact expiry boundary follows the documented policy
    (exclusive — expired at now == expires_at, covered by
    test_offer_expires_at_exclusive_boundary).
  • Expired offers cannot be accepted or mutated in unsafe ways
    (accept_offer fails closed; rejected calls leave storage untouched
    because Soroban rolls back state on contract errors).
  • Events expose the effective expiry and resulting state
    (OfferMade carries expires_at; OffAccpt/OfferCanc unchanged).
  • Boundary and property tests cover time progression and retries
    (exclusive-boundary accept/replace, past-boundary accept, expired-offer
    cancel, zero duration, u64::MAX overflow, expired re-offer).
  • Existing CI/CD checks remain green (pending CI run).
  • PR explains design, tradeoffs, test evidence, and limitations.

Validation

cargo test -p commitment-marketplace — focused coverage for the changed
behavior lives in tests.rs "Offer Expiration Policy Tests":
test_make_offer_zero_duration_fails,
test_make_offer_duration_overflow_fails,
test_offer_expires_at_exclusive_boundary,
test_expired_offer_replaced_by_reoffer. All existing make_offer call
sites were updated to pass an explicit 86400 duration.

Tradeoffs and limitations

  • Signature change: make_offer gains a required duration parameter;
    external callers must pass it. No in-repo callers existed outside unit
    tests and docs (all updated here).
  • Storage compatibility: stored Offer records written before this
    change lack expires_at and will not deserialize under the new layout;
    get::<_, Vec<Offer>> treats them as absent. Offers carry no escrowed
    funds (payment settles only at accept_offer), so a dropped stale offer
    strands no assets — the offerer simply re-offers.
  • Expired offers remain in storage until cancelled or replaced; no
    sweeper/background cleanup is added (keeps the change minimal).
  • get_offers returns expired offers too; liveness is derived by callers
    from expires_at versus ledger time.

Offers gain an expires_at ledger timestamp computed as created_at + duration (checked arithmetic). The boundary is exclusive - an offer is valid only while ledger.timestamp() < expires_at, matching the Auction::ends_at convention. Expired offers cannot be accepted (OfferExpired); re-offering after expiry replaces the stale offer; zero duration and arithmetic overflow fail with InvalidDuration. The OfferMade event exposes the effective expiry.

Fixes Commitlabs-Org#546
@woahwhattheheck

Copy link
Copy Markdown
Author

/claim #546

Original-author GrantFox OSS/FWC26 contribution and conditional compensation claim: I, @woahwhattheheck (GitHub account ID 293286387), submit this original contribution for GrantFox OSS Third Campaign issue #546 and affirmatively claim contributor credit and consideration for any eligible bounty compensation. Please confirm eligibility, acceptance criteria, award amount (if applicable), and payment instructions. The issue is labeled "Maybe Rewarded"; no award or payment is presumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Quality] Define offer expiration at exact ledger boundaries

1 participant