test(creator-keys): treasury rotation, sell TTL, blank handle and holder count coverage - #745
Open
ayinde38 wants to merge 1 commit into
Open
Conversation
…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.
|
@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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.rsCovers
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.rstests the read view andset_protocol_fee_recipient.rstests theset_entrypoint. Nothing covered theupdate_entrypoint's unauthorized/zero-address paths or the fee-routing consequence of a rotation.Naming note. The issue says
get_treasuryandinvalid_address; this contract calls themget_protocol_fee_recipientandContractError::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 addingassert_is_adminandvalidate_non_zero_addressto it and changing its signature to returnResult— 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 becauseset_treasury_addresscurrently has no admin check and no zero-address check — it callsadmin.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.rsThe 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:CREATOR_TTL_LEDGERSwindow, not merely "more than before"Removing the
extend_creator_ttlcall fromsell_keyfails these.#742 — blank display name
creator-keys/tests/display_name_empty_registration.rsThis one changes behaviour. A blank handle previously surfaced as
HandleTooShort(empty string, single space) orInvalidHandleCharacter(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 perdocs/error-extension-guide.mdso 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_MAXbuffer 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 stillHandleTooShort,"alice bob"is stillInvalidHandleCharacter.docs/error-codes.mddocuments the new variant.One existing test changed.
empty_handle_registration_regression.rs(#301) assertedHandleTooShortfor the empty string and now assertsDisplayNameEmpty. 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.mdexplicitly 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.rsholder_count_multiple_buyers.rscovers 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_supplyand 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 isget_creator_holder_count.Verification
cargo check --tests -p creator-keysandcargo clippy --tests -p creator-keysboth pass with zero warnings. I have not run the suite itself.