M2 review follow-ups: mainnet labels, deposit post-conditions, green CI#8
Open
jayteemoney wants to merge 3 commits into
Open
M2 review follow-ups: mainnet labels, deposit post-conditions, green CI#8jayteemoney wants to merge 3 commits into
jayteemoney wants to merge 3 commits into
Conversation
The app runs on mainnet but several UI surfaces still hardcoded
"testnet", telling visitors the wrong thing:
- landing CTA copy ("on Stacks testnet")
- footer Hiro Explorer link + bottom network link (?chain=testnet)
- sidebar network badge ("Testnet")
Derive all of these from the existing NETWORK config via a new
NETWORK_LABEL constant and the network-aware EXPLORER_BASE, so the
site always reflects the chain it is actually running on.
create-stream and top-up-stream are the calls where a user's funds leave their wallet, but both used PostConditionMode.Allow. Switch them to PostConditionMode.Deny with the existing explicit-amount post- condition bounding the sender's outflow, matching the deny-by-default exit paths (claim/cancel). The wallet now enforces the exact token and amount leaving the wallet at signing time.
The "status returns to ACTIVE after each resume" property test assumed every pause/resume cycle succeeds. When a random walk pushes the block height onto or past end-block between pause and resume, the contract correctly rejects resume with ERR-STREAM-ENDED (by design, to avoid a zombie ACTIVE state), leaving the stream PAUSED — so the assertion intermittently failed on streams that elapse mid-cycle. Break out of the cycle loop once pause or resume is no longer ok; the invariant only applies while the stream is still live. The deployed contract is unaffected — this was an over-strict test assertion.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Addresses the three follow-ups from the Milestone 2 review.
1. Mainnet labels (prioritized)
The app runs on mainnet, but several surfaces hardcoded "testnet" and told visitors the wrong thing. All now derive from the existing
NETWORKconfig via a newNETWORK_LABELconstant and the network-awareEXPLORER_BASE:?chain=testnet)No new env var needed — these track
NEXT_PUBLIC_NETWORK, which is alreadymainnetin production.2. Deny-by-default post-conditions on deposits
create-streamandtop-up-streamare the calls where a user's funds leave their wallet, but both usedPostConditionMode.Allow. Switched toPostConditionMode.Denywith the existing explicit-amount post-condition bounding the sender's outflow, matching the deny-by-default exit paths (claim/cancel).3. Green CI on main
The
multi-cycle pause/resumeproperty test ("status returns to ACTIVE after each resume") assumed every cycle succeeds. When the random walk lands the block height on/pastend-blockbetween pause and resume, the contract correctly rejects resume withERR-STREAM-ENDED(by design — prevents a zombie ACTIVE state), leaving the streamPAUSED, so the assertion flaked. The loop now breaks once pause/resume is no longerok; the invariant only applies while the stream is live. The deployed contract is unaffected — this was an over-strict test assertion.Verification
npm test— 125/125 passingtsc --noEmit— no errors in changed frontend files