feat(scripts): validate required fields in deployment manifests - #249
Merged
karagozemin merged 1 commit intoAug 31, 2026
Conversation
A manifest with every staking-asset identifier removed validated cleanly:
`resolverRegistryConfig` was checked only when present, so a deploy could ship
naming no asset contract, no minimum stake, and no slash beneficiary at all.
That block is now required, and each identifier inside it is checked by name.
The rules move out of the CLI into `scripts/lib/deployment-manifest.mjs`, a
read-only module that takes an already parsed manifest and touches neither the
filesystem nor the network. `validate-deployments.mjs` keeps its behaviour and
now only finds files, parses them, and prints what the validator reports.
Errors are `{ path, message }` pairs against the JSON path of the offending
field, and a field that already failed its type check is no longer reported a
second time for its network-specific value, so the output reads as a checklist
of what to fix.
Adds 26 tests covering a complete manifest, the manifest shipped in the repo,
and incomplete ones: missing network, chain, contract, and asset identifiers;
addresses from the wrong chain; malformed endpoints, dates, and transaction
hashes; and that every bad field is reported rather than only the first. Wired
as `pnpm test:scripts` and run in CI before the manifest gate itself.
Closes karagozemin#234
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016LpGd5DNEG334ZnjyNhmzm
|
@egekoca is attempting to deploy a commit to the karagoz's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@egekoca 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! 🚀 |
This was referenced Aug 31, 2026
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.
Closes #234
The gap
scripts/validate-deployments.mjsalready checked network and contract identifiers, but the staking-asset block was validated only when present:So a manifest that named no asset at all passed. Deleting
resolverRegistryConfigfromdeployments.testnet.json— removing the stake asset contract, the minimum stake, the admin, and the slash beneficiary — and running the validator onmaster:The resolver registry cannot run without those identifiers, and the manifest is the only place they are recorded.
There were also no tests. The rules lived inside a script that calls
main()on import and reads from disk, so they could not be exercised without a real file on the filesystem.The change
Asset identifiers are required.
resolverRegistryConfigmust be present, andstakeAsset,minStake,minStakeXLM,admin, andslashBeneficiaryare each checked by name. The same manifest now reports:The rules are now testable. They move into
scripts/lib/deployment-manifest.mjs, a read-only module that takes an already parsed manifest and touches neither the filesystem nor the network:validate-deployments.mjskeeps its exact CLI behaviour and now only finds files, parses them, and prints what the validator reports.One error per field. A field that already failed its type check is no longer reported a second time for its network-specific value — a missing
chainIdused to produce both "must be a finite number" and "must be 11155111 for testnet deployments". The output now reads as a checklist of what to fix.The shipped
deployments.testnet.jsonis unchanged and still validates.Tests
26 new tests (
scripts/lib/deployment-manifest.test.mjs,node --test, no dependencies).Valid manifests — a complete testnet manifest; the manifest actually shipped in the repository; optional blocks being absent.
Incomplete manifests — missing network, unknown network, network disagreeing with the file name, missing chain id, missing passphrase, an entire chain section absent; each Ethereum and Stellar contract missing by name; a missing
contractsblock; and every asset identifier above, including the case that regressed.Malformed values — an EVM address where a Soroban contract id belongs and the reverse, a Stellar account id where a contract id belongs, a non-http endpoint, a malformed date, a malformed transaction hash, and a manifest that is not an object.
Reporting — that three independent missing fields produce three errors in path order rather than only the first, and that
formatManifestErrorrendersfile:path message.CI
Added
pnpm test:scripts, run inci.ymlimmediately before theValidate deployment JSONstep — the validator's own tests pass before it is trusted to gate the manifest.🤖 Generated with Claude Code
https://claude.ai/code/session_016LpGd5DNEG334ZnjyNhmzm