-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dashboard | SDK] Feature: Ref values for address params in publish #5360
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5360 +/- ##
==========================================
- Coverage 45.50% 45.42% -0.09%
==========================================
Files 1071 1070 -1
Lines 55718 55835 +117
Branches 4032 4043 +11
==========================================
+ Hits 25357 25364 +7
- Misses 29674 29784 +110
Partials 687 687
*This pull request uses carry forward flags. Click here to find out more.
|
|
@@ -128,6 +129,187 @@ export type DeployContractfromDeployMetadataOptions = { | |||
salt?: string; | |||
}; | |||
|
|||
interface DynamicParams { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use type everywhere
paramValue: string | ImplementationConstructorParam; | ||
}; | ||
|
||
async function processRefDeployments( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add unit test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to its own file too pls
chain, | ||
account, | ||
contractId: contracts[0]?.contractId as string, | ||
publisher: contracts[0]?.publisherAddress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw clear err if none of those are defined
|
||
if (dynamicValue.type === "bytes[]") { | ||
const bytesArray = []; | ||
const decodedBytesArray = dynamicValue.decodedBytes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paramsToEncode ?
values.push(v.defaultValue); | ||
} | ||
|
||
if (v.dynamicValue) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else ?
bytesArray.push( | ||
encodeAbiParameters( | ||
types.map((t) => { | ||
return { name: "", type: t }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove name
} catch (e: any) { | ||
const err = "error" in e ? e.error?.message : e?.message; | ||
|
||
if (err.toString().includes("Contract already deployed")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
} | ||
} | ||
|
||
return JSON.stringify(bytesArray); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont think you need this?
@@ -159,6 +159,27 @@ type ParsedCompilerMetadata = { | |||
zk_version?: string; | |||
}; | |||
|
|||
type DynamicParams = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export this and re-use?
Problem solved
https://linear.app/thirdweb/project/[contract-tooling]-deployment-publish-flow-revamp-0194e04e2a84/overview
Handle contract refs in publish form -- Resolve / deploy all linked contracts in publish metadata, and pass the addresses as constructor args for corresponding params.
It recursively deploys all referenced contracts through the ref chain, before deploying the main contract.
PR-Codex overview
This PR focuses on enhancing the contract deployment process by introducing dynamic parameters, improving form handling for contract parameters, and refining the user interface for deploying contracts with references to other contracts.
Detailed summary
deploy-with-abi.ts
to return the address if already deployed.trusted-forwarders-fieldset.tsx
to cast trusted forwarders tostring
.DynamicParams
type indeploy-metadata.ts
for dynamic values.dynamicValue
support in multiple components for contract parameters.RefInputFieldset
andDecodedInputArrayFieldset
to manage dynamic inputs.ContractPublishForm
to support new implementation parameters.CustomContractForm
to handle dynamic values and references.