Skip to content

learn-token: storage size tracking, plus edge-case coverage for per-address minting, claim history, and pause - #373

Merged
DeFiVC merged 5 commits into
ChainLearnOfficial:mainfrom
oss-dw:feat/storage-size-tracking-251-254
Aug 30, 2026
Merged

learn-token: storage size tracking, plus edge-case coverage for per-address minting, claim history, and pause#373
DeFiVC merged 5 commits into
ChainLearnOfficial:mainfrom
oss-dw:feat/storage-size-tracking-251-254

Conversation

@bbjiggy

@bbjiggy bbjiggy commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Details

Prerequisite fixes (both pre-existing on main, unrelated to #251-254):

  1. storage.rs defined is_paused/set_paused twice — once from the original Emergency Pause PR (Feat/implement missing features #365) and again from the pause-events PR (feat: course content hash, per-address minting, claim history, pause events #363) — merged back to back without either noticing the other had added the same pair. Rust rejects this (E0428), so main doesn't compile. Removed the redundant pair.
  2. PR Feat/implement missing features #365 added Minter/Pauser role checks to mint(), pause(), and unpause() (an extra caller: Address param), and updated the call sites that existed at the time. It landed back to back with two other PRs (feat: course content hash, per-address minting, claim history, pause events #363, feat: governance voting, token vesting, permit gasless approval, and … #366) whose tests called the pre-refactor signatures or referenced an admin binding they never captured. None of the three PRs conflicted at the diff level, so all merged cleanly and cargo test -p learn-token has been failing to compile since. Fixed the affected call sites; no behavioral changes.

get_storage_size() (#254): increments/decrements around every write to a per-entity persistent key (reward claims, claim history, roles, whitelist, snapshots, vesting schedules/claims, proposals, votes, allowance-spender registries, permit nonces, per-address minting totals), guarded by a has() check so overwrites never double-count. Deliberately excludes singleton config (doesn't grow with usage) and Balance/Allowance (hottest write paths / temporary storage — see commit message for the full rationale).

Edge-case tests (#251, #252, #253): zero-amount mint leaves total_minted_to at 0 (only negative amounts are rejected); claim history stays correctly distinguished by course_id across different courses; pause blocks burn, claim_reward, and claim_vested in addition to the already-tested transfer/mint.

Test plan

  • cargo test -p learn-token — 78 passed, 0 failed
  • cargo test --workspace — all crates pass
  • cargo build --workspace — clean (one pre-existing, unrelated dead-code warning)

Closes #251, Closes #252, Closes #253, Closes #254

bbjiggy and others added 5 commits August 30, 2026 11:02
storage.rs defined is_paused/set_paused twice with identical bodies --
once under the original Emergency Pause section (ChainLearnOfficial#189) and again under
the pause-events section (ChainLearnOfficial#238), added by two PRs merged back to back
without either noticing the other had already added the same pair.
Rust rejects the duplicate definitions (E0428), so main currently
fails to build. Keep the original definitions and drop the redundant
pair; both events and pause/unpause logic already call through the
one storage.rs API surface, so nothing else changes.
PR ChainLearnOfficial#365 added Minter/Pauser role checks to mint(), pause(), and
unpause(), giving each an extra caller: Address parameter, and updated
the call sites that existed at the time. It landed back to back with
two other PRs (ChainLearnOfficial#363, ChainLearnOfficial#366) that added their own tests calling the
pre-refactor 2-and-0-argument signatures, or referencing an admin
binding some of those same tests never captured (destructured as _ or
_admin because they didn't need it before). None of the three PRs
conflicted at the git-diff level, so all three merged cleanly and the
test suite has been failing to compile since.

Capture admin from setup() wherever a fixed test now needs it as the
mint/pause/unpause caller, and fix one unrelated pre-existing
get_proposal(prop_id) call that needed a borrow. No behavioral changes
-- cargo test -p learn-token now compiles and all 63 tests pass.
Soroban prices persistent storage by entry count and size but gives a
contract no host API to enumerate or count its own keys, so there is
currently no way to answer "how much storage is this contract using"
short of an off-chain state export.

Add get_storage_size(), backed by a StorageEntryCount counter that
storage.rs increments/decrements around every write to a per-entity
key -- reward claims, claim history, roles, whitelist entries,
snapshots, vesting schedules and claimed amounts, proposals, votes,
allowance-spender registries, permit nonces, and per-address minting
totals -- guarded by a has() check so overwriting an existing entry
never double-counts it.

Deliberately excluded: singleton config (admin, name/symbol/decimal,
total/max supply, metadata, transfer restriction, wasm hash, upgrade
version, paused flag, proposal counter) since those don't grow with
usage, and Balance/Allowance since balances are written on every
transfer/mint/burn (the issue's "no performance impact" criterion
argues against adding a has() check to that path) and allowances live
in temporary rather than persistent storage.

Closes ChainLearnOfficial#254
…tory, and pause

total_minted_to() (ChainLearnOfficial#251), get_claim_history() (ChainLearnOfficial#252), and pause/unpause
(ChainLearnOfficial#253) were already implemented and tested on main (merged in ChainLearnOfficial#363 for
near-duplicate issues ChainLearnOfficial#236/ChainLearnOfficial#237/ChainLearnOfficial#238, before this repo's issue numbers
ChainLearnOfficial#251-253 were filed against the same scope). Add the edge cases their
existing coverage didn't reach:

- total_minted_to: a zero-amount mint is allowed (only negative amounts
  are rejected) and should leave the queryable total at 0.
- get_claim_history: existing tests only covered multiple quizzes
  within one course; verify records also stay correctly distinguished
  by course_id when a learner claims across different courses.
- pause: existing coverage only exercised transfer and mint; burn,
  claim_reward, and claim_vested are also gated by
  require_not_paused() and hadn't been exercised while paused.

Closes ChainLearnOfficial#251, Closes ChainLearnOfficial#252, Closes ChainLearnOfficial#253
@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@bbjiggy 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

@DeFiVC
DeFiVC merged commit 7bd8f69 into ChainLearnOfficial:main Aug 30, 2026
iduhtheman added a commit to iduhtheman/chainlearn-contracts that referenced this pull request Aug 30, 2026
…laim_record

A duplicate closing brace after append_claim_record's own closing
brace broke parsing for the entire learn-token crate, and therefore
the whole workspace (root-level integration tests depend on every
workspace member). Pre-existing on upstream/main since ChainLearnOfficial#373; needed
as a prerequisite to build and test anything in this workspace.
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.

. Add storage size tracking . Add contract pause events . Add reward claiming history . Add token supply tracking by address

2 participants