feat: expose unique trader count and emit UniqueTraderAdded (#985) - #987
Merged
Chucks1093 merged 5 commits intoSep 27, 2026
Merged
Chucks1093 merged 5 commits into
Chucks1093 merged 5 commits into
Conversation
The per-creator counting already existed: accrue_trade_analytics sets a HasTraded flag on a wallet's first buy or sell and increments UniqueTraderCount behind it, and get_analytics returns the total. What was missing was everything a caller could actually use. - events.rs: UniqueTraderAddedEvent (uniq_trd), carrying key_id, trader, the new count and the ledger. Published inside the first-trade branch, so an indexer gets exactly one event per wallet per creator rather than one per trade. - lib.rs: get_unique_trader_count(key_id) and has_traded(key_id, wallet) as public views. The count was previously reachable only by reading all three analytics fields; has_traded was not reachable at all, existing only as a storage-key helper. - test_unique_traders.rs: the two views, the event firing exactly once per wallet and once per new wallet, has_traded staying true after a wallet sells out, and a ten-wallet bulk-trade count. Closes accesslayerorg#985
|
@Yunusabdul38 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! 🚀 |
The verify job runs cargo fmt --all -- --check and flagged two hunks in the files added by the previous commit: the unique_trader_added_topics signature needed wrapping, and the wallets binding in test_unique_trader_count_after_bulk_trades fits on one line. No behaviour change.
…nto feat/unique-trader-tracking-985
Three problems in the tests added by the earlier commit, all caught by the verify job: - cargo fmt: unique_trader_added_topics needed a wrapped signature. - The crate is no_std, so std::vec::Vec did not resolve. Collect the bulk-trade wallets into a soroban_sdk::Vec instead. - Event topics come back as raw Val, which has no PartialEq, so comparing one against a Symbol did not compile. Convert the first topic to a Symbol before comparing; a non-symbol topic belongs to another event and does not match. The two event tests were also asserting against a running total, but env.events().all() only holds the most recent invocation's events. They now assert per call — one event on a wallet's first buy, none on its repeat buy or sell — which pins "exactly once per wallet" more tightly than the totals did. cargo fmt --all -- --check, cargo clippy --workspace --all-targets -D warnings, and cargo test --workspace all pass locally.
5 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
2 participants
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.
Summary
Exposes unique trader tracking on the bonding curve contract.
The counting itself already existed and is correct:
accrue_trade_analyticssets aHasTraded(creator, trader)flag on a wallet's first buy or sell and incrementsUniqueTraderCount(creator)behind it, so repeat trades do not double-count, andget_analyticsreturns the total. What was missing was everything a caller could use — a dedicated count view, a has-traded check, and an event.Changes
events.rs—UniqueTraderAddedEvent(uniq_trd), carryingkey_id,trader, the newunique_trader_countand the ledger, plus the matchingunique_trader_added_topicshelper.lib.rs— emits the event inside the first-trade branch, so an indexer gets exactly one event per(key_id, trader)pair rather than one per trade. Addsget_unique_trader_count(key_id)andhas_traded(key_id, wallet)as public views.test_unique_traders.rs— new test module.has_tradedpreviously existed only as a storage-key helper with no way to read it from outside the contract; the count was reachable only by fetching all three analytics fields.Acceptance criteria
test_unique_trader_count_view_matches_analytics.test_unique_trader_count_unchanged_by_repeat_tradesdrives two buys and a sell from one wallet and asserts the count stays at 1 whiletrade_countreaches 3.has_tradedreturns correct bool for traded and untouched wallets —test_has_traded_false_for_untouched_wallet,test_has_traded_true_after_first_buy,test_has_traded_is_per_wallet.test_has_traded_stays_true_after_selling_outpins that the flag records a trade having happened, not a balance being held.UniqueTraderAddedevent emitted exactly once per wallet —test_unique_trader_event_emitted_once_per_walletandtest_unique_trader_event_emitted_for_each_new_wallet.get_unique_trader_countreturns correct value after bulk trades —test_unique_trader_count_after_bulk_tradesruns ten wallets trading twice each and asserts 10 unique against 20 trades.One note on scope: the issue suggests storing the trader set "using a bitmap or sorted set pattern". The existing
HasTraded(creator, trader)flag already gives O(1) membership and insertion without materialising a set, and switching representations would rewrite working, tested accounting for no gain — so the storage layout is left as is.Closes #985
Closes #979
Closes #982
Closes #984