[Security Review] StackStream v1.0.0-rc1 — Pre-Mainnet Community Review #1

Activity

1. jayteemoney

   added a commit that references this issue on Apr 12, 2026

   Link community review issue #1 in SECURITY_REVIEW.md

   dbd0327
2. Sobilo34 commented on Apr 12, 2026

   

   Sobilo34

   on Apr 12, 2026

   More actions

   Independent review for StackStream Clarity contracts

   Scope: stream-manager.clar, stream-factory.clar — authorization, arithmetic, state transitions, token handling, aligned with SECURITY_REVIEW.md v1.0.0-rc1.

   Method: Manual trace of all public entry points; ran local simnet tests via npm install && npx vitest run (Vitest + Clarinet SDK).
   Summary: I agree with the bulk of the author review: use of contract-caller for access control, token principal equality checks, ordering of SIP-010 transfers vs map updates on create/top-up, cancel fund split (recipient-amount + sender-refund vs escrow), pause/resume accounting, and factory track-stream requiring the caller to be the stream’s sender. I did not identify a critical vulnerability (e.g. arbitrary drain or principal spoofing) in this codebase under standard SIP-010 assumptions.

   Finding non-zero rate-per-block invariant
   The document states that rate-per-block cannot be zero given deposit > 0 and duration > 0. With Clarity integer division, rate-per-block = floor((deposit * PRECISION) / duration) is zero whenever deposit * PRECISION < duration. Effects: no accrual for the recipient; top-up-stream divides by rate and fails for those streams. Funds are recoverable by the sender via cancel-stream, so this is not a direct theft bug, but it is a real edge case and breaks an implied invariant.

   Recommendation: Reject create-stream when (deposit-amount * PRECISION) < duration-blocks (equivalently require >= so the computed rate is at least 1). I implemented this guard and a regression test in my branch/PR: [link if applicable].
   Residual notes: Operator must protect CONTRACT-OWNER; emergency pause scope matches code. Third-party token behavior remains a deployment/integration risk.
3. Akanimoh12 commented on Apr 13, 2026

   

   Akanimoh12

   on Apr 13, 2026

   Last edited by Akanimoh12

   More actions

   Reviewer: Independent Review by Akanmoh Johnson
   Date: April 13, 2026
   Scope: stream-manager.clar (736 lines, 8 public functions), stream-factory.clar (218 lines, 4 public functions)
   References: SECURITY_REVIEW.md, Stacks Clarity Docs, Clarity tx-sender Security, Stacks Post-Conditions

   ---

   Executive Summary

   I performed a line-by-line review of both contracts against current Stacks/Clarity best practices (Clarity v3, Epoch 3.0) and the published self-audit. The overall design is solid — authorization, token conservation, and arithmetic safety are handled correctly. I found 0 Critical, 0 High, 0 Medium, 2 Low, and 3 Informational items. The self-audit's findings are confirmed and two additional items are reported below.

   ---

   Positive Observations (Best Practices Confirmed)

   Before the findings, credit where it's due — the following patterns align with current Stacks security best practices:

   Pattern	Status	Notes
   contract-caller for all authorization	✅ Correct	Prevents the well-documented tx-sender phishing attack where a malicious intermediary contract hijacks the persistence of tx-sender to bypass asserts! checks. Every mutating function in both contracts uses contract-caller, not tx-sender.
   stacks-block-height (not block-height)	✅ Correct	The contracts correctly use stacks-block-height, which is the Clarity 3 replacement. block-height is deprecated in Clarity 3 (Epoch 3.0) and returns tenure-height semantics instead of true Stacks block height. Using the wrong keyword would break all time-based calculations post-Nakamoto.
   Token substitution prevention	✅ Correct	Every function accepting a <sip-010-trait> parameter validates (is-eq token-principal (contract-of token)) against the stored principal. This prevents an attacker from passing a different token contract to drain the escrow.
   try! on all token transfers	✅ Correct	All contract-call? token transfer invocations are wrapped in try!, ensuring any transfer failure atomically reverts the entire transaction. No partial state can be written on transfer failure.
   State-after-transfer ordering	✅ Correct	In create-stream, the token escrow transfer executes before the stream data is written to maps. If the token transfer fails, no state is persisted. This is the right pattern even though Clarity has no reentrancy — it prevents state corruption from failed transfers.
   Arithmetic overflow safety	✅ Correct	The most sensitive product deposit-amount * PRECISION (where PRECISION = 1e12) stays well within Clarity's uint ceiling of 2^128 − 1 ≈ 3.4 × 10^38. For Bitcoin-scale tokens (21M × 10^8 decimals), the max product is ≈ 2.1 × 10^27. No practical overflow risk.
   Streamed amount clamp to deposit	✅ Correct	calculate-streamed-amount-internal ensures streamed ≤ deposit-amount, guarding against rounding-up edge cases.
   Emergency pause scoping	✅ Correct	set-emergency-pause blocks only create-stream. Existing streams continue to accrue, recipients can claim, and senders can cancel. This is the correct narrow scope — a circuit breaker should stop new exposure without freezing user funds.
   No reentrancy risk	✅ N/A	Clarity prohibits reentrancy at the language level. No mitigations required.
   Response handling	✅ Correct	All public functions return proper (response) types. Clarity enforces this at the consensus level — contracts with unchecked responses cannot be deployed.

   ---

   Findings

   Finding 1 — Rounding Dust Permanently Locked in Contract

   Function: create-stream / calculate-streamed-amount-internal
   Severity: Low
   Description:

   When deposit-amount is not perfectly divisible by duration-blocks, the rate calculation truncates:

   (rate-per-block (/ (* deposit-amount PRECISION) duration-blocks))

   After the stream fully elapses, the maximum streamed amount is:

   streamed = duration-blocks × floor(deposit-amount × 1e12 / duration-blocks) / 1e12
   

   Due to integer division, this can be strictly less than deposit-amount. The difference (dust) is permanently locked in the contract because:

   1. The recipient can never claim more than streamed (which is < deposit)
   2. The stream never reaches STATUS-DEPLETED (since withdrawn can never equal deposit)
   3. Only the sender calling cancel-stream can recover the dust (via sender-refund = deposit - streamed)

   Worked example:

   • deposit-amount = 10,000,000,000 (100 tokens, 8 decimals)
   • duration-blocks = 3
   • rate = 10,000,000,000 × 1e12 / 3 = 3,333,333,333,333,333,333
   • After 3 blocks: streamed = 3 × 3,333,333,333,333,333,333 / 1e12 = 9,999,999,999
   • Dust locked: 1 unit (0.00000001 tokens / 1 satoshi)

   For most real-world streams the dust is sub-satoshi and economically negligible. However, across thousands of streams, the cumulative locked dust grows.

   Suggestion:
   Option A (minimal): Document as a known limitation. Encourage senders to call cancel-stream after the stream fully elapses to recover residual dust.
   Option B (code change): In claim, when effective-elapsed == duration (stream fully elapsed), set streamed = deposit-amount instead of using the rate calculation. This guarantees the last claimable unit reaches the recipient.
   Option C: Add a sweep-dust admin function that can transfer residual amounts from fully-elapsed, non-cancelled streams back to the sender.

   ---

   Finding 2 — Stream Count Limit is Lifetime, Not Concurrent

   Function: create-stream / add-sender-stream / add-recipient-stream
   Severity: Low
   Description:

   The MAX-STREAMS-PER-USER (100) limit is enforced by sender-stream-count and recipient-stream-count, which are incremented on stream creation but never decremented when a stream is cancelled or depleted:

   ;; Incremented in add-sender-stream:
   (map-set sender-stream-count sender (+ current-count u1))
   
   ;; Never decremented anywhere in the contract

   This means the 100-stream cap is a lifetime limit per principal, not a concurrent limit. A DAO that creates 100 streams (even if all are subsequently cancelled or depleted) can never create stream #101.

   For individual users this is likely sufficient for v1. For high-frequency DAOs running recurring payroll streams (e.g., monthly contributor payments), this limit could be reached within ~8 years at one stream/month, or much sooner with weekly streams.

   Suggestion:
   Option A (minimal): Document the lifetime cap prominently. DAOs that anticipate high volume can rotate sender addresses.
   Option B (code change): Decrement the count in cancel-stream and in claim when the stream transitions to STATUS-DEPLETED. This turns the cap into a concurrent-streams limit.

   ---

   Finding 3 — Redundant Status Checks in pause-stream

   Function: pause-stream
   Severity: Low (confirming self-audit finding)
   Description:

   Lines 386–387 check status != CANCELLED and status != DEPLETED after line 385 already asserts status == ACTIVE. If status is ACTIVE, it cannot simultaneously be CANCELLED or DEPLETED. The checks are unreachable but cost gas.

   (asserts! (is-eq status STATUS-ACTIVE) ERR-STREAM-PAUSED)
   (asserts! (not (is-eq status STATUS-CANCELLED)) ERR-STREAM-CANCELLED)  ;; unreachable
   (asserts! (not (is-eq status STATUS-DEPLETED)) ERR-STREAM-DEPLETED)    ;; unreachable

   I confirm the self-audit's assessment: no security impact. Removing them saves execution cost and improves code clarity (no pun intended), but is not urgent for mainnet.

   ---

   Finding 4 — DAO Analytics Stale After Top-Up

   Function: track-stream (stream-factory.clar)
   Severity: Informational
   Description:

   When a DAO calls track-stream, the total-deposited analytics field is updated from the stream's deposit-amount at that moment:

   (map-set daos caller (merge dao-data {
     total-streams-created: (+ (get total-streams-created dao-data) u1),
     total-deposited: (+ (get total-deposited dao-data) (get deposit-amount stream))
   }))

   If the underlying stream is later topped up via top-up-stream (which increases deposit-amount and end-block), the factory's analytics are never refreshed. Additionally, track-stream can only be called once per stream (due to the ERR-ALREADY-TRACKED guard), so there is no way to re-read the updated deposit.

   Impact: Analytics only. No funds at risk. The stream-manager correct handles the top-up regardless of the factory's state.

   Suggestion: Accept as a known limitation for v1 analytics, or add a refresh-stream-tracking read-and-update function to stream-factory.

   ---

   Finding 5 — CONTRACT-OWNER Is Non-Transferable Single Key

   Function: set-emergency-pause
   Severity: Informational (confirming self-audit finding)
   Description:

   (define-constant CONTRACT-OWNER tx-sender)

   CONTRACT-OWNER is a constant set at deploy time. It cannot be updated to a multisig, a DAO, or a successor key. If the deployer key is lost, emergency pause capability is permanently lost. If compromised, the attacker can toggle the pause (but cannot access escrowed funds).

   I confirm the self-audit's assessment: acceptable for v1 given the narrow scope of the emergency function. The correct use of contract-caller (not tx-sender) in the check means the owner must call directly — a phishing contract cannot invoke set-emergency-pause on the owner's behalf by hijacking tx-sender.

   Suggestion for v2: Consider a two-step ownership transfer pattern (propose-new-owner / accept-ownership) or multisig governance.

   ---

   State Transition Analysis

   I traced all possible state machine paths:

                     create-stream
                         │
                         ▼
                  ┌─── ACTIVE ───┐
                  │      │        │
             pause-stream │   cancel-stream
                  │      │        │
                  ▼      │        ▼
               PAUSED    │    CANCELLED
                  │      │    (terminal)
            resume-stream│
                  │      │
                  └──────┘
                         │
                      claim (when withdrawn == deposit)
                         │
                         ▼
                     DEPLETED
                    (terminal)
   

   All transitions are correct:

   • ACTIVE → PAUSED: pause-stream (sender only). Only active streams can be paused.
   • PAUSED → ACTIVE: resume-stream (sender only). Pause duration correctly accumulated in total-paused-duration.
   • ACTIVE/PAUSED → CANCELLED: cancel-stream (sender only). Token conservation holds: recipient-amount + sender-refund = deposit - withdrawn (escrow balance).
   • ACTIVE → DEPLETED: claim when new-withdrawn == deposit. Automatic status transition.
   • CANCELLED/DEPLETED: Terminal states. All mutating functions correctly reject these.
   • Claiming while paused: Correctly allowed. calculate-effective-elapsed freezes time at paused-at-block, so the recipient claims only what accrued before the pause. This prevents a sender from holding earned tokens hostage.

   ---

   Arithmetic Verification

   Rate Calculation Precision

   rate = deposit × 1e12 / duration
   streamed = elapsed × rate / 1e12
   

   The 1e12 precision multiplier provides 12 additional decimal places of precision beyond the token's native decimals. For 8-decimal tokens (sBTC), this gives 20 effective decimal places in intermediate calculations — more than sufficient.

   Pause Duration Accumulation

   Across N pause/resume cycles:

   total_paused = Σ (resume_block[i] - pause_block[i])   for i in 1..N
   adjusted_elapsed = raw_elapsed - total_paused
   

   Since stacks-block-height is monotonically increasing, each pause_duration = current_block - paused_at_block ≥ 0. Underflow is impossible. The accumulation is correct across arbitrary pause cycles.

   Cancel Token Conservation

   recipient_amount = streamed − withdrawn        (unclaimed earned tokens)
   sender_refund    = deposit − streamed           (unstreamed tokens)
   Total outgoing   = (streamed − withdrawn) + (deposit − streamed) = deposit − withdrawn
   Contract holds   = deposit − withdrawn          ✓ Conservation holds
   

   Top-Up Rate Preservation

   additional_blocks = top_up_amount × 1e12 / rate
   

   Since rate = deposit × 1e12 / duration, substituting gives additional_blocks = top_up_amount × duration / deposit. The streaming rate (tokens per block) remains constant. The same rounding dust issue from Finding 1 applies to the extended portion.

   ---

   Summary Table

   #	Function	Severity	Description	Self-Audit
   1	create-stream / rate calc	Low	Rounding dust permanently locked when deposit % duration ≠ 0	Not covered
   2	create-stream / stream count	Low	100-stream limit is lifetime, not concurrent	Not covered
   3	pause-stream	Low	Redundant status checks (no security impact)	✅ Confirmed
   4	track-stream (factory)	Informational	DAO analytics stale after top-up	Not covered
   5	set-emergency-pause	Informational	Single non-transferable owner key	✅ Confirmed

   No critical, high, or medium severity issues found.

   ---

   Conclusion

   StackStream's contracts demonstrate strong security engineering for a v1 Clarity protocol. The authorization model correctly uses contract-caller throughout (the #1 Clarity security pattern), token conservation is maintained on every exit path, and the arithmetic handles precision responsibly. The use of Clarity 3 (stacks-block-height, Epoch 3.0) is correct for post-Nakamoto deployment.

   The two new findings (rounding dust lock and lifetime stream cap) are both Low severity and neither blocks mainnet launch. They are worth documenting and addressing in a future version.

   Good work preparing this for community review — the self-audit in SECURITY_REVIEW.md is thorough and accurate. I look forward to the mainnet launch.
4. Marvy247

   added a commit that references this issue on Apr 13, 2026

   security: apply community review fixes from issue jayteemoney#1

   ...

   e66917e
5. Marvy247 commented on Apr 13, 2026

   

   Marvy247

   on Apr 13, 2026

   More actions

   Completed an independent review of stream-manager.clar and stream-factory.clar. No critical, high, or medium findings.

   Function: claim
   Severity: Low
   Description: Authorization check (asserts! caller == recipient) was placed after the let block computed effective-elapsed, streamed, and claim-amount. A non-recipient caller triggers
   unnecessary computation before being rejected.
   Suggestion: Reorder asserts — authorization first, then state/token checks.

   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   Function: top-up-stream
   Severity: Low
   Description: Same pattern — auth check placed after let computed additional-blocks, new-deposit, new-end-block.
   Suggestion: Assert authorization before arithmetic bindings where possible.

   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   Function: resume-stream
   Severity: Low
   Description: No end-block guard. A sender can resume a paused stream after its natural end time, leaving it in STATUS-ACTIVE indefinitely. No extra tokens accrue (elapsed is clamped to
   duration), but the on-chain state is misleading until the recipient calls claim-all.
   Suggestion: Add (asserts! (< current-block end-block) ERR-STREAM-ENDED).

   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   Function: track-stream (stream-factory)
   Severity: Informational
   Description: total-deposited is recorded at tracking time and not updated when a stream is topped up. Analytics will undercount for topped-up streams.
   Suggestion: Add a sync-stream-deposit function in v2.

   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   Fixes for all three Low findings submitted in PR #3.
6. Ali6nXI commented on Apr 13, 2026

   

   Ali6nXI

   on Apr 13, 2026

   More actions

   The emergency pause only blocks new stream creation but still allows claims and cancels. Was this intentional design choice? Any scenario where you might want to also pause claims in a true emergency?
7. Godbrand0 commented on Apr 13, 2026

   

   Godbrand0

   on Apr 13, 2026

   More actions

   Independent review — novel finding in top-up-stream

   Function: top-up-stream
   Severity: Low
   Description: When the top-up amount is smaller than the stream's per-block rate in token units, integer division truncates additional-blocks to zero. The sender's tokens are transferred to the contract escrow, but end-block is unchanged. Because rate-per-block is never modified, the maximum tokens the recipient can ever claim is bounded by the original stream's duration × rate — the topped-up tokens silently exceed that ceiling and are permanently unreachable by the recipient until the sender calls cancel-stream.

   The condition is: amount × PRECISION < rate-per-block, equivalently amount < deposit / duration.

   Worked example:

   deposit-amount = 1_000_000_000 (1,000 tokens, 6 decimals)
   duration-blocks = 100
   rate-per-block = 1,000,000,000 × 1e12 / 100 = 1e22

   top-up amount = 5_000_000 (5 tokens)
   additional-blocks = (5_000_000 × 1e12) / 1e22 = 5e18 / 1e22 = 0

   Result:

   • 5 tokens transferred to escrow ✓
   • end-block unchanged — no extension
   • new-deposit = 1_005_000_000
   • max streamed over lifetime = 100 × rate / PRECISION ≈ 1_000_000_000
   • 5_000_000 tokens stuck in escrow, claimable only by sender via cancel-stream
     This is the direct top-up-stream analogue of the zero-rate guard already present in create-stream (line 225):

   ;; create-stream — guard present
   (asserts! (>= (* deposit-amount PRECISION) duration-blocks) ERR-INVALID-DURATION)

   ;; top-up-stream — analogous guard MISSING
   (additional-blocks (/ (* amount PRECISION) rate)) ;; silently truncates to 0
   Tokens are not permanently lost — the sender can recover them via cancel-stream — but the function silently accepts a no-op deposit instead of rejecting it, which is a real footgun for any DAO building payroll tooling on top of this protocol.

   Suggestion: Add the following assert in top-up-stream before the token transfer, mirroring the existing create-stream guard:

   ;; Prevent zero-extension top-ups: amount must extend the stream by at least 1 block
   (asserts! (>= (* amount PRECISION) rate) ERR-INVALID-AMOUNT)
   This rejects top-up amounts that are too small to extend the stream, forcing the caller to either top up with a meaningful amount or accept that very small additions aren't supported at the current rate.
8. dannyy2000 commented on Apr 13, 2026

   

   dannyy2000

   on Apr 13, 2026

   More actions

   Function: pause-stream / general design

   Severity: Medium

   Description: After the L-4 fix (which correctly blocks resume-stream past end-block), a new stuck-funds path exists. If a sender pauses a stream and never resumes or cancels it — and end-block passes — the unearned portion of the deposit is permanently locked in the contract with no recipient-side recovery path.

   Concretely:

   • Stream: deposit = 1000, duration = 100 blocks, start = 100, end = 200
   • Sender pauses at block 150 (half-way through)
   • Sender loses keys / goes silent
   • Block 200 passes — end-block is reached
   • Recipient can call claim and receives the 500 tokens earned before block 150
   • The remaining 500 tokens are stuck forever: resume-stream fails with ERR-STREAM-ENDED, cancel-stream requires the sender, and set-emergency-pause only blocks new stream creation — it has no authority over existing stream escrows

   The only recovery path (sender calling cancel-stream) belongs exclusively to the sender. No admin escape hatch exists for this scenario.

   Suggestion: Several options exist at different trade-off points:

   1. (Minimal change) Add an expire-stream public function callable by anyone once stacks-block-height > end-block and status == STATUS-PAUSED. It would settle identically to cancel-stream — earned tokens to recipient, remainder back to sender — without requiring the sender to act.

   2. (Recipient-friendly) Allow the recipient to call a claim-all-on-expiry that releases the full remaining escrowed balance to themselves once stacks-block-height > end-block + grace-period and the stream is still paused.

   3. (Design change) Extend end-block in resume-stream by the pause duration (new-end-block = end-block + pause-duration). This eliminates the scenario entirely: streams that were paused always get their full intended streaming window, so expiry-while-paused never leaves a shortfall.
9. dannyy2000 commented on Apr 13, 2026

   

   dannyy2000

   on Apr 13, 2026

   More actions

   Function: pause-stream

   Severity: Low

   Description: pause-stream does not validate that current-block >= start-block (i.e. that the stream has actually started). A sender can pause a stream before streaming begins. When they later call resume-stream, the full wall-clock pause duration — including time before start-block — is added to total-paused-duration. This pre-start time is meaningless (no tokens were accruing), but it still reduces the effective streaming window.

   Example:

   • Stream: start = 200, end = 300, deposit = 100, duration = 100
   • Sender pauses at block 190 (10 blocks before start)
   • Sender resumes at block 260
   • pause-duration = 260 − 190 = 70 is stored in total-paused-duration
   • At block 300 (end-block): raw-elapsed = 100, adjusted-elapsed = 100 − 70 = 30
   • Recipient can only claim 30 tokens, though the stream was genuinely active from 260 → 300 (40 blocks)
   • 10 tokens are permanently unclaimable (recoverable only via cancel-stream)

   The 10-block discrepancy comes from the pre-start pause time being overcounted.

   Suggestion: Add a start-block guard in pause-stream:

   (asserts! (>= current-block (get start-block stream-data)) ERR-INVALID-START-TIME)

   This prevents pausing a stream that has not yet started and eliminates the overcounting.
10. zachyo commented on Apr 13, 2026

    

    zachyo

    on Apr 13, 2026

    More actions

    Completed an independent review of stream-manager.clar and stream-factory.clar. No critical or high findings. One medium and two low findings.

    ---

    Function: cancel-stream

    Severity: Low

    Description: cancel-stream is callable by the sender even while the stream is STATUS-PAUSED. When paused, calculate-effective-elapsed correctly freezes elapsed time at paused-at-block.
    This means a sender can: (1) create a stream, (2) immediately pause it before the recipient claims anything, (3) cancel it and recover nearly the entire deposit minus only the blocks elapsed before the pause. This is technically correct behaviour, but it makes streams a reversible commitment. The sender retains unilateral ability to claw back funds at any time by pausing then cancelling, giving recipients very weak guarantees.

    Suggestion: This is a design decision, not a bug. But it should be prominently documented as a trust assumption: recipients must trust the sender not to pause-cancel. For v2, consider a non-cancellable flag or a minimum lock-in period. At minimum, document that recipient funds are only guaranteed once the stream elapses (or they claim).

    ---

    Function: set-emergency-pause

    Severity: Medium

    Description: CONTRACT-OWNER is set to tx-sender at deploy time (the top-level define-constant). In Clarity, tx-sender at the top level of a contract is the deployer's address. If this contract is ever redeployed (e.g. upgraded), and deployed by a different address or a deployment script, CONTRACT-OWNER silently changes to whatever address ran the deploy transaction. There is no on-chain visibility into who owns the contract outside of reading the constant. Additionally, the current self-audit notes this is a single non-transferable key but what's missing is that there is no transfer-ownership function, making key rotation impossible without redeployment (which changes all contract addresses and breaks all existing integrations).

    Suggestion: For mainnet, at minimum store the owner as a define-data-var with a transfer-ownership function guarded by the current owner. This enables key rotation without redeployment and is a standard pattern in Clarity admin contracts.

    ---

    Function: top-up-stream

    Severity: Low

    Description: top-up-stream does not check status == ACTIVE explicitly — it only blocks CANCELLED and DEPLETED. This means a paused stream can be topped up, which extends its end-block. This is not necessarily wrong, but it creates an edge case: after the top-up, the extended end-block may now be far in the future, but the resume-stream guard (< current-block end-block) will pass — so a stream that was paused past its original end-block and would have been unresumable can now be resumed by topping up first. This is a sneaky state bypass.

    Suggestion: Either explicitly allow pause-state top-ups (and document it) or add (asserts! (not (is-eq status STATUS-PAUSED)) ERR-STREAM-PAUSED). The former is more flexible but needs documentation. If allowed, consider whether the end-block extension should account for the ongoing pause duration.
11. IdokoMarcelina commented on Apr 14, 2026

    

    IdokoMarcelina

    on Apr 14, 2026

    More actions

    Security Review - StackStream Contracts
    Function: create-stream
    Severity: Medium
    Description: You validate the recipient address, but there's nothing stopping someone from sending tokens to a malicious or non-compliant contract.
    Suggestion: Consider maintaining a list of approved SIP-010 compliant tokens, or at minimum document the risk so other developers are aware.

    Function: top-up-stream
    Severity: High
    Description: The calculation (/ (* amount PRECISION) rate) will cause a division by zero if rate is ever zero. While the current create-stream logic makes a zero rate unlikely, there is no explicit guard in top-up-stream itself.
    Suggestion: Add (asserts! (> rate u0) ERR-INVALID-DURATION) before the division.

    Function: pause-stream
    Severity: Informational
    Description:
    Error reuse: (asserts! (is-eq status STATUS_ACTIVE) ERR-STREAM-PAUSED)
    This returns ERR-STREAM-PAUSED even if stream is cancelled or depleted, which can be misleading.

    Suggestion:
    Use more precise error codes for better debugging and UX.
12. Ryjen1 commented on Apr 15, 2026

    

    Ryjen1

    on Apr 15, 2026

    More actions

    Function: claim-all
    Severity: Informational
    Description: The function hardcodes the maximum uint value directly when calling claim. This large magic number reduces code readability and could become a maintenance issue if Clarity's uint behavior or the internal min logic changes in the future.
    Suggestion: Define a clear constant such as (define-constant MAX-CLAIM-AMOUNT u340282366920938463463374607431768211455) at the top of the file for better maintainability.

    Function: transfer-ownership
    Severity: Low
    Description: Ownership transfer (via updating the contract-owner data-var) is immediate and one-step. There is no confirmation step, so a typo or wrong address results in permanent loss of owner privileges with no recovery mechanism.
    Suggestion: Implement a standard two-step ownership transfer: add propose-new-owner and accept-ownership functions.

    General Finding – Event Logging
    Severity: Informational
    Description: Events are emitted consistently using (print {...}), but none of the events include a version field or schema identifier. Future contract upgrades could silently break off-chain indexers, dashboards, and analytics tools.
    Suggestion: Add an event-version: "v1.0.0" (or similar) field to all printed events and document the current schema version in the README
13. Jayy4rl commented on Apr 15, 2026

    

    Jayy4rl

    on Apr 15, 2026

    More actions

    Stream-Manager Contract

    1. Function: create-stream

    Severity: Informational
    Description: The guard on line 227 (asserts! (>= (* deposit-amount PRECISION) duration-blocks) ERR-INVALID-DURATION) is well-designed to prevent zero rate-per-block scenarios. This correctly enforces the invariant that rate-per-block >= 1, preventing silent math failures.
    Suggestion: No changes needed. Document this constraint in README for clarity.

    ---

    2. Function: create-stream

    Severity: Informational
    Description: The contract uses contract-caller for authorization (line 231), which correctly captures the actual transaction sender for role-based checks. This is appropriate for access control decisions.
    Suggestion: Consider documenting the authorization model (contract-caller vs tx-sender rationale) in code comments.

    ---

    3. Function: claim

    Severity: Low
    Description: On line 323, the claim amount is silently clamped to available balance without warning: (claim-amount (if (> amount claimable) claimable amount)). While not a security issue, users requesting more than available may not realize they're getting less.
    Suggestion: Consider emitting an event with both requested and actual claim amounts for transparency.

    ---

    4. Function: claim

    Severity: Informational
    Description: Token conservation is correctly enforced via:

    • Claimable amount never exceeds streamed amount (line 320)
    • Streamed amount clamped to deposit amount in calculate-streamed-amount-internal (line 158-160)
    • Withdrawn-amount updated atomically (line 342-346)

    All three exit paths (partial claim, full claim, cancellation) maintain the invariant: withdrawn-amount + unearned-refund = deposit-amount.

    ---

    5. Function: pause-stream

    Severity: Low
    Description: Redundant state checks on lines 391-393:

    (asserts! (is-eq status STATUS-ACTIVE) ERR-STREAM-PAUSED)
    (asserts! (not (is-eq status STATUS-CANCELLED)) ERR-STREAM-CANCELLED)
    (asserts! (not (is-eq status STATUS-DEPLETED)) ERR-STREAM-DEPLETED)

    The first assertion already requires status == STATUS-ACTIVE, so the second and third are unreachable. This has no security impact.
    Suggestion: Remove lines 392-393 for code clarity.

    ---

    6. Function: resume-stream

    Severity: Informational
    Description: The check on line 444 (asserts! (< current-block (get end-block stream-data)) ERR-STREAM-ENDED) correctly prevents resumes after the stream window closes, eliminating the "zombie ACTIVE state" scenario.
    Suggestion: No changes needed. This is correct-by-design.

    ---

    7. Function: cancel-stream

    Severity: Informational
    Description: Token conservation verified:

    • Recipient gets: streamed - withdrawn (unclaimed earned portion)
    • Sender gets: deposit - streamed (unearned portion)
    • Total returned: (streamed - withdrawn) + (deposit - streamed) = deposit - withdrawn ✓
    • State update correctly sets withdrawn-amount to streamed to prevent double-claiming (line 526)

    ---

    8. Function: expire-stream

    Severity: Informational
    Description: Permissionless expiry is correctly gated by three preconditions:

    1. Stream must be paused (line 589)
    2. Current block >= end-block (line 592)
    3. Token must match (line 595)

    This prevents abuse while enabling recovery from stuck-paused-and-expired scenarios.

    ---

    9. Function: top-up-stream

    Severity: Informational
    Description: Zero-extension prevention guard on line 675 (asserts! (>= (* amount PRECISION) rate) ERR-INVALID-AMOUNT) correctly prevents "silent no-op" scenarios where tokens transfer to escrow but end-block doesn't extend. This is well-designed defensive programming.

    ---

    10. Function: calculate-effective-elapsed

    Severity: Informational
    Description: Pause duration accumulation correctly handles:

    • Active state: uses current-block for elapsed (line 122, 126-128)
    • Paused state: freezes elapsed at paused-at-block (line 126-128)
    • Accumulated pauses subtracted from raw elapsed (line 136-138)
    • Clamped to max duration (line 141-143)

    No arithmetic overflow risks identified. All comparisons use <=/>= appropriately.

    ---

    11. Function: calculate-streamed-amount-internal

    Severity: Informational
    Description: High-precision math implemented correctly:

    • Formula: (effective-elapsed * rate-per-block) / PRECISION (line 155)
    • Numerator: effective-elapsed * rate-per-block can safely fit in uint (max ~2^128)
    • Division by PRECISION downscales to actual tokens ✓
    • Result clamped to deposit (line 158-160) prevents rounding artifacts

    ---

    12. Function: set-emergency-pause

    Severity: Informational
    Description: Emergency pause is correctly scoped: stops new stream creation (line 214) but doesn't freeze existing streams. Recipients can still claim, senders can still cancel. This is a reasonable circuit breaker design.

    ---

    Stream-Factory Contract

    13. Function: register-dao

    Severity: Informational
    Description: Authorization is implicit via contract-caller (line 64). DAO admin = caller. This is correct for self-registration.

    ---

    14. Function: track-stream

    Severity: Informational
    Description: Cross-contract call to stream-manager (line 157) is correctly followed by re-verification that caller == stream.sender (line 160). This prevents a malicious actor from tracking someone else's stream.

    ---

    15. Function: track-stream

    Severity: Low
    Description: No idempotency guard for repeated tracking. If the same stream is tracked multiple times, total-deposited increments each time (line 171). However, the guard on line 163 (asserts! (is-none ...)) prevents this issue—attempting to re-track throws ERR-ALREADY-TRACKED.

    ---

    16. Function: update-dao-name

    Severity: Informational
    Description: Name registry collision detection on line 106 (asserts! (is-none (map-get? dao-names new-name))) correctly prevents name reuse. Old name properly deleted on line 109 before reassignment.

    ---

    SUMMARY

    Severity	Count
    Critical	0
    High	0
    Medium	0
    Low	2 (redundant checks in pause-stream; minor UX on claim silencing)
    Informational	14

    All findings are either no-ops or minor code-clarity improvements. The authorization model (contract-caller for all mutating calls), token conservation, arithmetic safety, and state transitions are solid. The contract demonstrates excellent defensive programming (zero-rate guards, zero-extension prevention, pause boundary checks).
14. jayteemoney

    closed this as completedon Apr 15, 2026