feat(campaign-escrow): add per-campaign applicant index and paged view - #79
Merged
JamesVictor-O merged 2 commits intoAug 29, 2026
Merged
Conversation
`cargo fmt --all -- --check` has been failing on main since 43ba011, 7c41d08 and 748ba2f landed on 2026-08-25, so the CI fmt job is red on every open PR rather than on anything those PRs changed. Pure rustfmt output on the four reported hunks — two call-site rewrappings in lib.rs and two assert_eq! expansions in test.rs. No semantic change. Unrelated to the dispute-resolution TTL fix in this branch; kept as its own commit so it can be dropped or cherry-picked independently if the maintainers would rather land it separately.
Applications are keyed by DataKey::Application(campaign_id, creator), so reading a campaign's applicants required already knowing every creator's address. There was no way to enumerate them, which is what the business dashboard's applicant review panel needs. Add DataKey::CampaignApplicant(campaign_id, ordinal) -> Address: one fixed-size entry per applicant, written at apply time next to the existing ApplicantCount increment. This deliberately does not restore the Vec<Address> that Ads-Bazaar#43/Ads-Bazaar#50 removed. A single growing vector — or a chunked page of them — is rewritten on every apply, so its write cost climbs with the number of prior applicants. Indexing by ordinal keeps both writes fixed-size, and the regression test Ads-Bazaar#50 added, applying_with_many_prior_applicants_does_not_regress_write_cost, still passes unmodified. Expose two views: applicant_count(campaign_id) -> u32 campaign_applicants(campaign_id, start, limit) -> Vec<Address> campaign_applicants is paged rather than all-at-once. One entry per applicant means a page of N costs N of footprint, and the first cut at a 100-wide page failed on Soroban's footprint limit with "total footprint ledger entries: 103 > 100". MAX_APPLICANTS_PAGE is 50, which leaves headroom for the campaign, count and instance entries. A start at or past the end returns empty rather than erroring, so clients can page until they get a short read. Both views resolve the campaign first, so an unknown campaign reports CampaignNotFound instead of being indistinguishable from one with no applicants. The AlreadyApplied guard the issue also asks for already exists, via an O(1) get_application lookup — left as is rather than rewritten to scan the new index. A test asserts a rejected double-apply leaves no duplicate ordinal behind. Closes Ads-Bazaar#8
7 tasks
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.
Problem
Closes #8.
Applications live under
DataKey::Application(campaign_id, creator), so reading a campaign's applicants requires already knowing every creator's address. There is no way to enumerate them, which is what the business dashboard's applicant review panel needs.Why this doesn't restore the
Vec<Address>the issue asks forThe issue predates #43/#50. #50 removed a
CampaignApplicantsVec<Address>precisely because it was rewritten on everyapply_to_campaign, making the cost of applying grow with the number of prior applicants, and it shippedapplying_with_many_prior_applicants_does_not_regress_write_costto hold that line — an exactassert_eq!onwrite_bytesbetween the first and the 202nd apply.That assertion rules out any growing container, including a chunked/paginated
Vec: a partly-filled page writes fewer bytes than a full one, so the equality breaks. (Worth noting the Vec was never there to serve this issue anyway — #38 added it purely forupdate_campaign_metadata's boolean "has anyone applied?" check.)So the index is keyed by ordinal instead:
One fixed-size entry per applicant, written at apply time alongside the existing
ApplicantCountincrement. Both writes are fixed-size, so applying costs the same whether you are the 1st or the 10,000th applicant. #50's regression test passes unmodified — I did not touch it.New API
Two deliberate deviations from the signature in the issue:
It's paged. One ledger entry per applicant means a page of N costs N entries of footprint. My first cut used a 100-wide page and it failed outright on Soroban's footprint limit:
MAX_APPLICANTS_PAGEis therefore 50, leaving headroom for the campaign, count and contract-instance entries. An unpagedcampaign_applicants(campaign_id) -> Vec<Address>as literally specified would compile and pass a 3-applicant test, then fail in production on exactly the popular campaigns whose applicants a business most needs to review. Astartat or past the end returns empty rather than erroring, so clients page until they get a short read.Both views resolve the campaign first, so an unknown campaign reports
CampaignNotFoundrather than being indistinguishable from a campaign with no applicants — a bare count can't express that difference.The index is status-agnostic by design, as the issue specifies: pair it with
get_application(campaign_id, creator)to read each applicant's status. That keeps filtering a client concern and keeps this function's cost independent of how the applications are doing.On the remaining acceptance criteria
AlreadyAppliedguard — already present atlib.rs:360via an O(1)get_applicationlookup, andError::AlreadyAppliedalready exists aterror.rs:23. Left as-is rather than rewritten to scan the new index; the direct key lookup is strictly cheaper.double_apply_same_creatoralready covers it, and I've added a test that a rejected retry leaves no duplicate ordinal behind.get_campaign_applicantbumps on read. These entries are written once and never rewritten, so reads are the only thing that can refresh them; without a read bump a stable applicant list would decay out from under an index that is only ever read.Test coverage added
New
test_campaign_applicantsmodule (7 tests):three_creators_apply_and_are_returned_in_order— the issue's stated criterion.applicants_page_across_multiple_calls_without_gaps_or_repeats— walks 7 applicants in pages of 3 the way a client would, asserting the reassembled list equals application order exactly.limit_is_capped_at_max_page— an over-largelimitis clamped, and the remainder is still reachable from the next offset. This is the test that caught the footprint bug above.start_at_or_past_end_returns_empty_rather_than_erroringcampaign_with_no_applicants_returns_empty_and_zero_countunknown_campaign_is_distinguishable_from_no_applicantsrejected_double_application_is_not_indexed_twiceTest plan
cargo test --workspace— 151 pass (105 escrow, up from 98; 18 integration; 28 dispute-resolution)applying_with_many_prior_applicants_does_not_regress_write_costpasses unmodified — the constant-write-cost property from perf: CampaignApplicants storage grows unboundedly for a boolean check #43/fix: replace CampaignApplicants Vec with O(1) ApplicantCount storage #50 is preservedcargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleancargo build --workspace --target wasm32v1-none --release— buildsNote on the first commit
cargo fmt --all -- --checkis currently failing onmain(four hunks incampaign-escrowfrom 43ba011, 7c41d08, 748ba2f on 2026-08-25), so the fmt job is red on every open PR. d65d999 here applies rustfmt so this PR's CI is green; it's the same commit as in #78, so it will drop out cleanly once either merges.