Fix #8013#8014
Conversation
🦋 Changeset detectedLatest commit: 06a5833 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR fixes a verification regression where an execution state’s contractName can already be a fully-qualified name (FQN), and adds a regression test + fixture to ensure the generated verify target doesn’t duplicate the source name.
Changes:
- Add a new regression test and mock deployment fixture covering
contractNameprovided as an FQN. - Fix
getVerificationInformationto avoid prefixingartifact.sourceNamewhencontractNameis already an FQN.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| v-next/ignition-core/src/verify.ts | Avoids generating source:source:Contract when contractName is already an FQN. |
| v-next/ignition-core/test/verify.ts | Adds regression coverage for FQN contractName in verification info generation. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/journal.jsonl | Mock journal containing a library deployment where contractName is stored as an FQN. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/deployed_addresses.json | Mock deployed addresses corresponding to the new fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/build-info/254cc89254e61cb6a9bf2c74f13055f1.json | Build info needed by the mock deployment fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/artifacts/LockModule#WAAIT.json | Artifact for the contract in the new fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/artifacts/LockModule#WAAIT.dbg.json | Debug artifact pointing to build-info for the new fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/artifacts/LockModule#UUUUU.json | Artifact for the library in the new fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/artifacts/LockModule#UUUUU.dbg.json | Debug artifact pointing to build-info for the new fixture. |
| v-next/ignition-core/test/mocks/verify/fqn-contract-name/README.md | Documents the purpose of the new mock fixture. |
Comments suppressed due to low confidence (2)
v-next/ignition-core/test/verify.ts:95
- The comment has a stray quote and reads awkwardly ("Note""). Consider changing it to something like "Note: the library in this test is called UUUUU" to avoid confusion.
// Note" the library in this test is called UUUUU
assert(typeof info !== "string", "Expected a VerifyInfo, got a string");
v-next/ignition-core/test/mocks/verify/fqn-contract-name/README.md:1
- Grammar in this README sentence is off (missing a verb). Consider rephrasing to something like "This mock deployment contains a library deployed with an FQN as its contract name."
This mock deployment a library deployed with a FQN as contract name.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR adds a regression test for #8013 based on an existing test, and then fixes it in a second commit.
I also manually verified that the fix works in the reproduction linked in #8013