chore: dump fixtures without rewriting the global Solana config - #720
Open
MarkFeder wants to merge 1 commit into
Open
chore: dump fixtures without rewriting the global Solana config#720MarkFeder wants to merge 1 commit into
MarkFeder wants to merge 1 commit into
Conversation
Every `prepare.mjs` that pulls a program fixture from mainnet ran `solana config set -um` first, which writes mainnet into `~/.config/solana/cli/config.yml` and leaves it there. A plain `pnpm install` was therefore enough to silently repoint a developer's CLI, and the `deploy` scripts in these examples take no cluster flag, so a later deploy would follow it to mainnet. Passing `-um` to `solana program dump` scopes the cluster to that one command instead. Verified for both script styles (zx and stdlib) with the global config on devnet: the fixtures still download from mainnet and `solana config get` is unchanged afterwards.
Contributor
Greptile SummaryThis PR scopes mainnet selection to each fixture download instead of mutating the developer’s global Solana CLI configuration.
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness, security, or compatibility issues identified. The changes consistently replace a persistent global CLI mutation with a command-scoped cluster option, and the supplied verification confirms both invocation styles still download the expected fixture without altering the configured cluster. Important Files Changed
Reviews (1): Last reviewed commit: "chore: dump fixtures without rewriting t..." | Re-trigger Greptile |
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.
Repo-wide follow-up to a review finding on #719, in the same spirit as #702.
The problem
Every
prepare.mjsthat pulls a program fixture from mainnet ran this first:That is not a per-command flag — it writes mainnet into
~/.config/solana/cli/config.ymland leaves it there. Sopnpm installin any of these examples silently repoints the developer's CLI at mainnet, and since thedeployscripts here take no cluster flag, a laterpnpm deployfollows it.The fix
Drop the global mutation and pass
-umto the dump itself, so the cluster is scoped to the one command that needs it:8 files, two script styles (zx in seven, Node stdlib in
nft-operations/pinocchio). No behavioural change to what gets downloaded.The
deployscripts keep taking no cluster flag on purpose — they should deploy wherever the developer has pointed their CLI. That is only correct onceprepare.mjsstops moving the target underneath them.Verification
Ran both script styles in a container with the global config deliberately set to devnet:
tokens/create-token/anchor(zx, viapnpm install) →token_metadata.sodownloaded, 793,991 bytestokens/nft-operations/pinocchio(stdlib, vianode prepare.mjs) → same fixture, same sizesolana config getstill reported devnet after bothprettier --checkpasses on all 8 files.