Repository navigation
feat(stream): add transfer_recipient to hand off an active stream - #253
Conversation
|
@Jayking40 is attempting to deploy a commit to the techalhaji's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Jayking40 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! 🚀 |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
resolve conflicts please. |
…ards (#247) This PR resolves the following assigned issues: - closes #78 - closes #83 - closes #81 - closes #79 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 `main` had already shipped in 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=100` vs `start=500`); the `now == start_time` and `now == end_time` cases did not exist. Four tests pin the boundary semantics: - `test_cancel_at_exactly_start_time_refunds_full_amount` — at `now == start_time` the refund is the whole deposit (`vested_amount` is 0 at the start instant), `claimable == 0`, and a follow-up `withdraw` fails with `NothingToWithdraw`. - `test_cancel_at_exactly_end_time_refunds_nothing` — at `now == end_time` nothing is refunded (`vested_amount` returns 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_exactly` and `test_cancel_at_midpoint_truncated_division_no_drift` — a mid-schedule cancel refunds exactly `total - vested`, including a truncating schedule (1000 over 3 ledgers: refund 667 / vested 333) with `refund + payout == locked total` and 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`/`vested` read amplification (`contracts/stream/src/lib.rs`, +29, of which 4 lines are a `#[cfg(test)] extern crate std;` the cost test's `println!` needs in this `no_std` crate)** 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 the `vesting_summary` view compute both from a **single** storage read. It reuses `vested_amount()` and `(vested - withdrawn).max(0)`, i.e. literally the bodies of `claimable` (`contracts/stream/src/lib.rs:590-599`) and `vested` (`:602-609`), so the two paths cannot drift apart. - `test_claimable_vested_view_costs` asserts the resource counts through `Env::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 with both dedicated views at every schedule point, partially withdrawn and cancelled streams) pin correctness. ## What the rebase removed (and why) - **#79 (hostile-token tests) — dropped, no `closes` line.** `main` merged #254, which shipped the shared `HostileTokenContract` fixture (`contracts/test-support/test_support.rs`), 5 escrow + 2 splits + 5 stream reentry tests, and the SECURITY.md *Threat Model* + *Token trust assumptions* write-up. The pre-rebase branch had its own parallel `HostileTokenHandle` + `reentry()` + `set_shortfall` scaffolding and 9 `test_hostile_token_*` tests, and a 76-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.rs` and `contracts/test-support/test_support.rs` are all back to `main`'s content byte-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. - **#81 (milestone vesting) — split out, no `closes` line.** Feature-sized work does not belong on a test/perf branch. `Tranche` / `MilestoneSet` / `DataKey::Milestones` / `create_milestones` / `release_milestones` / `cancel_milestones` / `get_milestones`, the six new `StreamError` variants and their 7 tests are parked for a dedicated PR. This PR keeps only `docs/milestone-vesting.md`, the design note for that follow-up, because no other lane 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`. - **#280 overlap check.** `main`'s unified status/progress derivation lives in `packages/types/src/index.ts` and `apps/web`; `stream/src/lib.rs` gained only `create_streams`, `top_up`, `pause`, `resume`, `version` and the fee/treasury/pause events. Nothing on the contract side already returned `(vested, claimable)` together (`grep -n vesting_summary` on `main` → no hits), so `vesting_summary` is not a duplicate and was kept. - Third-party #253 touches `contracts/stream/src/{lib,test}.rs`; this branch was resolved against `main` only and never rebased onto that PR. The overlap is disclosed rather than coordinated. ## Test results Gates run locally (this is a fork PR whose `ci.yml` runs wait for maintainer approval, so nothing here is a CI claim). Head `1413bd40`, base `fb1b9294`: ``` cargo fmt --manifest-path contracts/Cargo.toml --all -- --check -> exit 0 cargo test -p stream --manifest-path contracts/Cargo.toml -- --nocapture -> test result: ok. 52 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (45 pre-existing on main + the 7 new tests above, each listed and run by name) cargo clippy -p stream --manifest-path contracts/Cargo.toml --all-targets -- -D warnings -> exit 0 Measured in test_claimable_vested_view_costs: budget cpu claimable=67743 vested=67743 summary=69569 ``` `pnpm`-side jobs were not re-run and are not claimed: the branch differs from `main` in no `.ts`/`.tsx`/`.json` package file (`git diff --name-only fb1b929 HEAD` covers only `contracts/stream/**` and `docs/milestone-vesting.md`), so `typecheck`/`lint`/`test`/`test:exports` consume byte-identical inputs to `main`. 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 are the first-ever golden for those test names and a `cargo test` run needs them. No pre-existing snapshot file is changed: the 17 goldens that drift additively on every local run (`paused_at`/`total_paused` fields `main` never regenerated after #182) and the 4 goldens missing on `main` for its own pause/event tests are left untouched, outside this diff, exactly as inherited.
…recipient-to-hand-off-an-active-stream
|
conflict resolved, kindly review and merge |
…recipient-to-hand-off-an-active-stream
…recipient-to-hand-off-an-active-stream
|
Please merge PR, I have resolve conflict about 4 times |
…recipient-to-hand-off-an-active-stream
|
ci is failing |
Earlier merges from main dropped code main had added (fee, treasury and pause events, batch and pause tests, SDK failover helpers), which broke every CI job. This re-applies the transfer_recipient feature on top of current main: - SameRecipient moves to error code 20; 12-19 are taken on main. - transferRecipient follows main's invoke(method, args, opts) signature and accepts WriteOptions. - SorobanClient gains a protected publicKey getter now that options is private. - The SDK test uses fakeTransport, like the rest of main's suite. - Stream bindings regenerated with Stellar CLI 27.0.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Ci is fixed, boss please merge now thanks |
feat(stream): add
transfer_recipientto hand off an active streamSummary
A stream's recipient had no way to move a stream to a new address after a key rotation or a sale. This PR adds
transfer_recipientto the stream contract. Only the current recipient can call it, and all future withdrawals go to the new address.Signature note: the function takes an explicit
callerargument:transfer_recipient(stream_id, caller, new_recipient). A plainrecipient.require_auth()rejects other signers with a host auth trap, not a typed error. The acceptance criteria requireUnauthorized, so this follows the escrow contract's pattern:callermust sign and must match the stored recipient.Changes
transfer_recipientUnauthorized.callerwithout their signature still fails the auth check.withdrawn,total_amountand the schedule stay the same, so past withdrawals stay where they are. Anything not yet withdrawn, including tokens that vested but were never claimed, becomes withdrawable by the new address only. The docs tell recipients towithdrawfirst if they want to keep those tokens.StreamRecipientTransferredevent, published as("stream", "transferred", id)withold_recipientandnew_recipientin its data.SameRecipienterror (code 12), appended to the enum. Transferring to the current recipient fails instead of emitting a misleading event.Streamstruct is unchanged. The struct's spec entry is byte-identical to before, so existing SDK decoding keeps working.StreamsClient.transferRecipient(id, newRecipient). It uses the connected wallet ascallerand checks the new address before any network call.SAME_RECIPIENTadded to the error codes.Testing
withdrawnand the schedule are kept, and the final balances.cancel, moving the vested remainder.Unauthorizedfor both a third party and the sender.SameRecipient, andStreamNotFoundfor an unknown stream.UNAUTHORIZEDandSAME_RECIPIENTbefore the wallet is asked to sign.cargo fmt --checkandcargo clippy -p stream -D warningsare clean.test_fee_cap_boundaryalready fails onmain. It's unrelated to this change and left as is.Closes #183