feat: registering to aztec registry on deployment - #102
Conversation
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
3 similar comments
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Benchmark Comparison
Contract: counter
|
||||||||||||||||||||||||||||||||||||||||||||||||||
…plate into feat/contract-registry
5faec71 to
b2fd1e3
Compare
There was a problem hiding this comment.
✨ PR Review
The PR adds artifact registry upload functionality during contract deployment. The implementation includes appropriate environment variable configuration and best-effort upload by default. However, there are two significant issues with error handling and deployment flow that should be addressed.
2 issues detected:
🐞 Bug - String values from failed JSON parsing are incorrectly returned as response objects via unsafe type assertions.
Details: When the server returns a successful response (200 or 409) with non-JSON content, safeJsonParse returns the raw string, which is then type-cast to ArtifactRegistryUploadResponse. This causes the function to return a string instead of a proper response object, breaking the contract for callers expecting an object with a 'success' field.
File:src/ts/artifactRegistry.ts🐞 Bug - Function throws after successful deployment in strict mode, hiding the deployed contract and violating atomic operation principles. 🛠️
Details: The artifact upload occurs after contract deployment completes. In strict mode, if the upload fails, the contract is already deployed but the function throws an error. This creates an inconsistent state where the caller receives an error but the deployment actually succeeded, potentially leading to confusion, lost contract references, or duplicate deployments on retry.
File:src/ts/utils.ts (31-34)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
| await maybeUploadArtifactToRegistry({ | ||
| artifact: CounterContractArtifact, | ||
| filename: "counter_contract-Counter.json", | ||
| }); |
There was a problem hiding this comment.
🐞 Bug - Deployment State Inconsistency: Either move the upload before deployment (if feasible), or catch upload errors and return the contract with a warning/metadata about upload failure instead of throwing. Alternatively, clearly document this behavior and consider returning both the contract and upload result in a structured response.
| await maybeUploadArtifactToRegistry({ | |
| artifact: CounterContractArtifact, | |
| filename: "counter_contract-Counter.json", | |
| }); | |
| try { | |
| await maybeUploadArtifactToRegistry({ | |
| artifact: CounterContractArtifact, | |
| filename: "counter_contract-Counter.json", | |
| }); | |
| } catch (uploadError) { | |
| console.warn("Failed to upload artifact to registry:", uploadError); | |
| } |
Is this review accurate? Use 👍 or 👎 to rate it
If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over
|
Closing this PR as non required feature. |
✨ PR Description
Purpose: Add Aztec artifact registry integration to automatically upload compiled contract artifacts to devnet.aztec-registry.xyz during deployment for improved contract discoverability.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how