-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
docs: proxy contract cookbook #3253
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
CodSpeed Performance ReportMerging #3253 will not alter performanceComparing Summary
|
…/chore/manual-proxy-contracts
…FuelLabs/fuels-ts into db/chore/manual-proxy-contracts
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.
Awesome work mate, just some typos:
|
||
[dependencies] | ||
standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.6.0" } | ||
sway_libs = { git = "https://github.com/FuelLabs/sway-libs", tag = "v0.24.0" } |
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.
We must be sure this is the correct version of this contract.
Can we please have a README explaining all these details?
This must always be paired with the one used by the fuels
CLI, and forc
:
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.
You'll notice we use a pre-built version in the' fuels' CLI, which is the same approach followed by forc
:
Perhaps we could even reuse the same built version inside the fuels/deploy
command. We'd only need the source to use the contract's contents in code snippets, which doesn't seem to be the case.
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.
@arboleya Should we export the generated types from the Src14Proxy
contract in the umbrella package?
This way, we can advise users who prefer manual deployment ( not using the fuels CLI ) to use our exported version, ensuring it stays in sync with the audited version.
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.
@Torres-ssf @arboleya I get a cyclic dep if I export this directly from fuels
, do we want, dare I say, another package?
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.
We could export it from fuel-ts/abi-typegen
as example standards?
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.
Exporting this has been a bit painful.
If the generated factory exists inside fuels
and we want to export it from there, we get cyclic deps for all the things that the factory itself is importing. So we'd need to swap out the imports for the module imports.
It probably needs to exist in it's own package, that imports all the required module imports that the factory needs. With a script that swaps the fuels
imports for the module imports so we can still export it from fuels
.
Will feedback soon.
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.
@danielbate Yes. I've struggled to find a solution for this as well.
Going to delay this till after #3369 to not impede snippet finalisation. |
…/chore/manual-proxy-contracts
Coverage Report:
Changed Files:
|
Release notes
In this release, we:
Checklist