Skip to content

test(creator-keys): treasury rotation, sell TTL, blank handle and holder count coverage - #745

Open
ayinde38 wants to merge 1 commit into
accesslayerorg:mainfrom
ayinde38:test/creator-keys-740-741-742-743
Open

test(creator-keys): treasury rotation, sell TTL, blank handle and holder count coverage#745
ayinde38 wants to merge 1 commit into
accesslayerorg:mainfrom
ayinde38:test/creator-keys-740-741-742-743

Conversation

@ayinde38

Copy link
Copy Markdown

Closes #740
Closes #742
Closes #743
Closes #741

Four test suites, plus the one behaviour change #742 needs in order to have something to assert against.

#740 — protocol fee recipient update persistence

creator-keys/tests/protocol_fee_recipient_update_persistence.rs

Covers update_protocol_fee_recipient: the rotated address is what actually lands on the ledger (asserted by reading the storage key directly, not only through the view), the superseded address is gone, fees accrued after the rotation are payable to the new recipient, and both guards reject without disturbing the stored value.

Existing coverage was adjacent but not overlapping — protocol_fee_recipient.rs tests the read view and set_protocol_fee_recipient.rs tests the set_ entrypoint. Nothing covered the update_ entrypoint's unauthorized/zero-address paths or the fee-routing consequence of a rotation.

Naming note. The issue says get_treasury and invalid_address; this contract calls them get_protocol_fee_recipient and ContractError::ZeroAddress. Tests use the real names.

I deliberately did not target set_treasury_address (the literal "treasury" entrypoint), because satisfying the issue's "non-admin panics" and "zero address panics" criteria there would mean adding assert_is_admin and validate_non_zero_address to it and changing its signature to return Result — an ABI change touching ~10 call sites across 5 existing test files. That looks like its own issue rather than something to smuggle into a test PR. Flagging it because set_treasury_address currently has no admin check and no zero-address check — it calls admin.require_auth() but never verifies the caller is the admin, so anyone can set the protocol treasury to any address including the zero address. Happy to open a separate issue if that's useful.

#741 — TTL extension on sell

creator-keys/tests/ttl_extension_on_sell.rs

The buy path's TTL extension was well covered; the sell path had a single "TTL went up" assertion inside ttl_extension_on_buy.rs. Adds the rest:

  • a successful sell restores a full CREATOR_TTL_LEDGERS window, not merely "more than before"
  • repeated sells reset the window from the current ledger rather than stacking on the remainder (asserted with both a lower and an upper bound, which is what makes this meaningful)
  • a sell that reverts on slippage extends nothing
  • a sell by a wallet with no balance, which reverts before ever reaching the extension call, extends nothing

Removing the extend_creator_ttl call from sell_key fails these.

#742 — blank display name

creator-keys/tests/display_name_empty_registration.rs

This one changes behaviour. A blank handle previously surfaced as HandleTooShort (empty string, single space) or InvalidHandleCharacter (three spaces) — neither of which tells the caller what actually went wrong, and the two are inconsistent with each other for what a user would consider the same mistake.

Adds ContractError::DisplayNameEmpty = 40, appended per docs/error-extension-guide.md so no existing discriminant moves, and checks it ahead of the length and character rules.

The over-length check still runs first, because the handle bytes are read into a fixed HANDLE_LEN_MAX buffer and reordering that would be a buffer overrun. Documented in the function.

The other handle rules are unchanged, and the tests pin that: "aa" is still HandleTooShort, "alice bob" is still InvalidHandleCharacter. docs/error-codes.md documents the new variant.

One existing test changed. empty_handle_registration_regression.rs (#301) asserted HandleTooShort for the empty string and now asserts DisplayNameEmpty. The rejection itself and the "no state written" invariant that test guards are both unchanged — only the variant is more specific. docs/error-extension-guide.md explicitly calls for updating nearby tests when adding a variant.

#743 — holder count across a buy/sell sequence

creator-keys/tests/holder_count_buy_sell_sequence.rs

holder_count_multiple_buyers.rs covers three wallets holding one key each — which an implementation that counted keys rather than wallets would also pass. Adds the case that separates them: wallets holding several keys, where partial sells must leave the count alone and only the final key may decrement it. Also covers repeat buys by an existing holder and re-entry after a full exit.

Every step cross-checks the view against get_creator_supply and the per-wallet balances, so the count can't drift from the state it's derived from.

Naming note. The issue says get_holder_count; the view is get_creator_holder_count.

Verification

cargo check --tests -p creator-keys and cargo clippy --tests -p creator-keys both pass with zero warnings. I have not run the suite itself.

…holder count

Adds the four test suites requested in accesslayerorg#740, accesslayerorg#741, accesslayerorg#742 and accesslayerorg#743, plus the
one behaviour change accesslayerorg#742 needs to have something to assert against.

protocol_fee_recipient_update_persistence.rs (accesslayerorg#740)
  Covers update_protocol_fee_recipient: the rotated address is what lands on
  the ledger, the superseded one is gone, fees accrued after the rotation are
  payable to the new recipient, and both guards (non-admin, zero address)
  reject without disturbing the stored value.

ttl_extension_on_sell.rs (accesslayerorg#741)
  The buy path's TTL extension was covered; the sell path only had a single
  "TTL went up" assertion. Adds the full set: a successful sell restores a
  complete CREATOR_TTL_LEDGERS window, repeated sells reset that window from
  the current ledger rather than stacking on the remainder, and a sell that
  reverts — on slippage, or with no balance — extends nothing.

display_name_empty_registration.rs (accesslayerorg#742)
  A blank handle previously surfaced as HandleTooShort (empty, single space)
  or InvalidHandleCharacter (three spaces), neither of which tells the caller
  what actually went wrong. Adds ContractError::DisplayNameEmpty = 40 —
  appended per docs/error-extension-guide.md, so no existing discriminant
  moves — and checks it ahead of the length and character rules. The other
  handle rules are unchanged and the tests pin that: "aa" is still
  HandleTooShort and "alice bob" is still InvalidHandleCharacter.

  empty_handle_registration_regression.rs (accesslayerorg#301) asserted HandleTooShort for
  the empty string and now asserts DisplayNameEmpty. The rejection and the
  "no state written" invariant it guards are unchanged.

holder_count_buy_sell_sequence.rs (accesslayerorg#743)
  Existing coverage has three wallets holding one key each, which a key
  counter would also pass. Adds the case that separates the two: wallets
  holding several keys, where partial sells must leave the count alone and
  only the final key may decrement it. Also covers re-entry after a full exit.
  Every step cross-checks the view against supply and per-wallet balances.

docs/error-codes.md documents the new variant.
@drips-wave

drips-wave Bot commented Aug 25, 2026

Copy link
Copy Markdown

@ayinde38 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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