Repository navigation
perf(stream): single-read vesting_summary view and cancel-boundary guards - #247
Merged
techAlhaji merged 2 commits intoOct 20, 2026
Merged
Conversation
|
@Ayomikun2005 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! 🚀 |
|
@Ayomikun2005 is attempting to deploy a commit to the techalhaji's projects Team on Vercel. A member of the Team first needs to authorize it. |
Collaborator
|
resolve conflicts please. |
Issue SorobanForge#83: callers that need both `vested` and `claimable` paid two full reads of the same stream entry. `vesting_summary` returns the pair from a single read; `test_claimable_vested_view_costs` pins the resource counts so a regression that re-reads the entry fails. Issue SorobanForge#78: the cancel suite only exercised mid-schedule and pre-start timestamps. Four tests pin `now == start_time`, `now == end_time` and the midpoint boundaries, including a truncating schedule where refund + payout must equal the locked total exactly. Scope: rebased onto main, which already carries the hostile-token work (SorobanForge#254) and the unified status/progress derivation (SorobanForge#280), so this branch keeps only what those do not provide. docs/milestone-vesting.md is the design note for issue SorobanForge#81, implemented in a follow-up PR.
Ayomikun2005
force-pushed
the
fix-issues-78-79-81-83
branch
from
October 20, 2026 14:37
657b6b5 to
1413bd4
Compare
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.
This PR resolves the following assigned issues:
Rebased onto current
main(fb1b9294, "refactor(types): unify stream status and progress derivation" #280).The branch now carries exactly the two things this issue pair is about; everything that
mainhad already shippedin the meantime was dropped rather than re-fought (see "What the rebase removed").
Changes Made
#78 — cancel at schedule boundaries (
contracts/stream/src/test.rs, +127, appended at end of file)The suite cancelled at 30 % of the schedule and strictly before start (
ts=100vsstart=500); thenow == start_timeandnow == end_timecases did not exist. Four tests pin the boundary semantics:test_cancel_at_exactly_start_time_refunds_full_amount— atnow == start_timethe refund is the whole deposit(
vested_amountis 0 at the start instant),claimable == 0, and a follow-upwithdrawfails withNothingToWithdraw.test_cancel_at_exactly_end_time_refunds_nothing— atnow == end_timenothing is refunded (vested_amountreturns the total once
now >= end_time), the recipient withdraws the full amount and the contract drains to zero.test_cancel_at_midpoint_refunds_total_minus_vested_exactlyandtest_cancel_at_midpoint_truncated_division_no_drift— a mid-schedule cancel refunds exactlytotal - vested,including a truncating schedule (1000 over 3 ledgers: refund 667 / vested 333) with
refund + payout == locked totaland no residue.No off-by-one was found, so the linear path in the contract is unchanged; the behaviour is now locked in by tests.
Existing tests were not renumbered or reflowed — the block is appended after
test_version_matches_shared_constant.#83 —
claimable/vestedread amplification (contracts/stream/src/lib.rs, +29, of which 4 lines are a#[cfg(test)] extern crate std;the cost test'sprintln!needs in thisno_stdcrate)Neither view re-reads the stream entry redundantly internally: each touches exactly the instance + stream entry
and writes nothing (asserted). The genuine redundancy is across the pair — a dashboard that wants both numbers paid
two full entry reads and two invocations.
VestingSummary { vested, claimable }and thevesting_summaryview compute both from a single storage read.It reuses
vested_amount()and(vested - withdrawn).max(0), i.e. literally the bodies ofclaimable(
contracts/stream/src/lib.rs:590-599) andvested(:602-609), so the two paths cannot drift apart.test_claimable_vested_view_costsasserts the resource counts throughEnv::cost_estimate()(write entries,ledger-entry reads, and that the combined view is cheaper than the two views back to back), so a future regression
that reads the stream entry twice fails.
test_claimable_vested_boundary_values(before start / exactly start / exactly cliff / last ledger before end /exactly end / far past end, plus withdrawn bookkeeping) and
test_vesting_summary_matches_views(agreement withboth dedicated views at every schedule point, partially withdrawn and cancelled streams) pin correctness.
What the rebase removed (and why)
closesline.mainmerged fix(contracts): write state before payouts and test token reentrancy #254, which shipped the sharedHostileTokenContractfixture (contracts/test-support/test_support.rs), 5 escrow + 2 splits + 5 stream reentrytests, and the SECURITY.md Threat Model + Token trust assumptions write-up. The pre-rebase branch had its own
parallel
HostileTokenHandle+reentry()+set_shortfallscaffolding and 9test_hostile_token_*tests, and a76-line Known Limitations section. Carrying them would have meant a second hostile fixture in shared test support
and a second, partly contradictory security doc, so
SECURITY.md,contracts/escrow/src/test.rs,contracts/splits/src/test.rsandcontracts/test-support/test_support.rsare all back tomain's contentbyte-for-byte and left the diff. Honest residual, left on the table and not claimed here: the merged fixture always
transfers the full amount, so the "create overstates the stream" / "withdraw underpays the recipient"
short-transfer scenarios still have no coverage. That needs a fixture change, not a re-add of mine.
closesline. Feature-sized work does not belong on a test/perfbranch.
Tranche/MilestoneSet/DataKey::Milestones/create_milestones/release_milestones/cancel_milestones/get_milestones, the six newStreamErrorvariants and their 7 tests are parked for adedicated PR. This PR keeps only
docs/milestone-vesting.md, the design note for that follow-up, because no otherlane owns that path yet; the doc now states explicitly at the top that the entry points are not implemented on
this branch, and its error-code numbers were corrected to the next free values after
AmountOverflow = 19.main's unified status/progress derivation lives inpackages/types/src/index.tsandapps/web;stream/src/lib.rsgained onlycreate_streams,top_up,pause,resume,versionand thefee/treasury/pause events. Nothing on the contract side already returned
(vested, claimable)together(
grep -n vesting_summaryonmain→ no hits), sovesting_summaryis not a duplicate and was kept.contracts/stream/src/{lib,test}.rs; this branch was resolved againstmainonly andnever rebased onto that PR. The overlap is disclosed rather than coordinated.
Test results
Gates run locally (this is a fork PR whose
ci.ymlruns wait for maintainer approval, so nothing here is a CI claim).Head
1413bd40, basefb1b9294:pnpm-side jobs were not re-run and are not claimed: the branch differs frommainin no.ts/.tsx/.jsonpackage file (
git diff --name-only fb1b9294 HEADcovers onlycontracts/stream/**anddocs/milestone-vesting.md), sotypecheck/lint/test/test:exportsconsume byte-identical inputs tomain.Snapshot note: the 7 goldens under
contracts/stream/test_snapshots/test/for the 7 new tests(
test_cancel_at_exactly_*,test_cancel_at_midpoint_*,test_claimable_vested_boundary_values,test_claimable_vested_view_costs,test_vesting_summary_matches_views, each.1.json) ARE committed — they arethe first-ever golden for those test names and a
cargo testrun needs them. No pre-existing snapshot file ischanged: the 17 goldens that drift additively on every local run (
paused_at/total_pausedfieldsmainneverregenerated after #182) and the 4 goldens missing on
mainfor its own pause/event tests are left untouched,outside this diff, exactly as inherited.