feat: publish contract addresses and a release manifest with each release - #242
feat: publish contract addresses and a release manifest with each release#242re-gius wants to merge 4 commits into
Conversation
CI Summary
Deploy ContractsDeployed addresses vs the committed manifestExpected is the committed manifest; actual is this CI deployment of the same pipeline.
Labelsdependencies, other, type: docs |
|
New release flow, documented in
A release that changes no contract code skips all of this. Step 3 is manual today. The next piece of work is for |
There was a problem hiding this comment.
Collapsing the three copies of the addresses into one tracked manifest and generating the rest from it fixes the drift from 6f1e6fb. Reading the factory out of the canonical manifest at CI time instead of repeating the literal means the two cannot disagree, and running validate on PRs fails a bad manifest before a release does. A few suggestions below, mostly non-blocking; the validate() tidy-up is the one worth doing.
| const contracts = contractsFromManifest(path); | ||
| log(`${network} (chain ${chainId}): ${Object.keys(contracts).length} addresses`); | ||
| } | ||
| const missing = readContractNames().filter( |
There was a problem hiding this comment.
Small thing: the contract list gets read and parsed here, then again on line 175 just to log the count. Reading it once into a variable and reusing it would avoid the second parse.
| log(`${network} (chain ${chainId}): ${Object.keys(contracts).length} addresses`); | ||
| } | ||
| const missing = readContractNames().filter( | ||
| (name) => !existsSync(join(ROOT, "out", `${name}.sol`, `${name}.json`)) && !sourceExists(name), |
There was a problem hiding this comment.
sourceExists runs a separate git ls-files for every contract name. On the new metadata job there is no forge build, so out/ is absent and all 32 listed contracts fall through to their own git call. Listing the tracked sources once and checking membership would do the same work more simply. I checked that a set built from the sources resolves the same 32 names as the per-name globs, so it is behaviour preserving. Happy to make the change if you would like it.
There was a problem hiding this comment.
Done in 824137d and thanks for checking. trackedSourceNames() now lists contracts once and builds a Set of basenames.
| cwd: ROOT, | ||
| encoding: "utf8", | ||
| }); | ||
| return found.trim() !== ""; |
There was a problem hiding this comment.
This catches any git failure and returns false, so a genuine git error surfaces as "no source or build artefact", which is a little misleading. It fails safe, so only a clarity point, and folding this into a single sources lookup avoids it anyway.
There was a problem hiding this comment.
Changed in 824137d - a real git failure reports itself now
| const changed = Object.keys(current).filter( | ||
| (name) => | ||
| previous[name] && | ||
| JSON.stringify(previous[name].contracts) !== JSON.stringify(current[name].contracts), |
There was a problem hiding this comment.
This compares the two address sets by serialising them. It is fine today because Foundry writes the manifests with sorted keys, so both sides serialise in the same order, but it does tie the "addresses changed" note to that ordering. If a manifest were ever hand-edited into a different key order, the notes would announce a move that did not happen. Comparing per address, or sorting keys first, would make it robust. Minor.
There was a problem hiding this comment.
Fixed in 824137d - sameAddresses() compares per label and case-insensitively now
| @@ -0,0 +1,370 @@ | |||
| #!/usr/bin/env bun | |||
There was a problem hiding this comment.
The file has a bun shebang but is always invoked as bun scripts/..., so the shebang is never used and the file is not executable. Worth a glance in case it was meant to run directly, otherwise fine to leave.
|
|
||
| Two ways to protect yourself. Resolve addresses through the protocol registry at runtime, so the chain is the authority and the published file is only a starting point. Or check a release against a chain yourself with `deployments:verify` before relying on it. | ||
|
|
||
| The manual step is deliberate for now, and this file stays the single source of truth for addresses either way. The next step is for a deployment to update it automatically, so recording addresses stops depending on someone remembering. Later, publishing can read the chain and refuse to publish addresses that disagree with it. |
There was a problem hiding this comment.
Optional: this is a roadmap note in a reference doc. If no issue tracks it, a reader cannot act on it. Drop it, or link the issue.
Description
Releases currently ship ABIs but no contract addresses, so a consumer gets the interface and still doesn't know how to call the contracts without scraping a doc or hardcoding a set. This publishes
deployments.json(addresses per network) andrelease-manifest.json(what the release contains) as standalone assets and at the root of the zip, generated from the committed deployment manifests. Both are in the pre-publish asset check, so a release either carries them or doesn't publish.It also removes the repo's two other copies of addresses: the 17-address list in
DEPLOYMENTS.md, which had already drifted once (6f1e6fbc), and thePINNED_FACTORYliteral indeploy-contracts.yml, now read from the manifest.deployments/<network>/<chain-id>.jsonbecomes the only tracked copy.New
scripts/js/release-metadata.mjs: it has no dependencies, usescastfor chain reads, withbuild,validate,changelogandverify. A small new workflow runsvalidateon PRs touchingdeployments/**or the contract list, so a duplicate address or malformed manifest fails there rather than when a release is cut.Type
Scope
Related Issues
Follows #222 , which made the ABI artifact trustworthy. This PR extends that artifact with addresses.
Fixes
Fixes #118 .
Checklist
Code
forge buildpassesforge testpassesTesting
Security
selfdestructordelegatecallDocumentation
Breaking Changes
Breaking changes: none for on-chain behaviour or for existing release assets, which keep their names and shapes. One thing to be aware of: anything scraping the address list out of
DEPLOYMENTS.mdwill no longer find it and should readdeployments/<network>/<chain-id>.jsonor the publisheddeployments.json.How to test
Not a contracts change, so
forge testexercises none of it. From a checkout of this branch:The end-to-end check that matters is a dry run of Publish Beta Package with a throwaway version, the same method used to validate #237. I will run it before merging it.
Notes
The manifest is updated by hand, and stays the single source of truth. A live deploy commits only its markdown report in dotns-releases; nothing writes addresses back here. The next step after this merges is for dotns-releases to open that PR automatically. RELEASE_ARTIFACTS.md states this plainly rather than implying a mechanism that doesn't exist. The manifest is accurate today, the latest report for each live network reproduces the canonical set with no differences.