-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
feat: create and deploy a reference proxy contract for contracts with [proxy]
enabled
#6069
base: master
Are you sure you want to change the base?
Conversation
Benchmark for 946beecClick to view benchmark
|
Benchmark for 52d495eClick to view benchmark
|
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.
Looking good! Left a few initial comments/questions
Benchmark for 6828faaClick to view benchmark
|
07910ec
to
7d3613e
Compare
Benchmark for 82fee6fClick to view benchmark
|
Benchmark for 1101331Click to view benchmark
|
Benchmark for c157f2bClick to view benchmark
|
Benchmark for 126ba37Click to view benchmark
|
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.
A few suggestions for the docs, but this is a great feature. The auto updating of the proxy address in the manifest is great
5ad5d69
to
a29d8a3
Compare
Benchmark for 7d4734aClick to view benchmark
|
chore: use defaults from constants chore: clippy fix feat: update proxy addres after new deployment of it refactor: remove duplicate password prompts and cleanup secret key selection chore: add remove todo comment feat: actually update proxy target if there is an address in the forc.toml clippy fix test: add proxy contract build test docs: update docs markdown lint fix comment typo docs: add proxy docs fix docs lints update docs from review Co-authored-by: K1-R1 <[email protected]> Co-authored-by: Sophie Dankel <[email protected]> pin to tag
a29d8a3
to
9f406fd
Compare
I had to rebase as there were conflicts. |
Benchmark for 0a46d9bClick to view benchmark
|
Benchmark for b441f2eClick to view benchmark
|
Benchmark for 4fab154Click to view benchmark
|
Benchmark for 7c5e50cClick to view benchmark
|
I tried this out on The first deployment was successful, and the Forc.toml was updated correctly. The second time I tried deploying, I got this error:
However, there are sufficient funds in the account. Blocked by FuelLabs/fuels-rs#1396 The other issue is that, for the second request, it's not clear to the user what they are signing. There should be a message like |
Benchmark for 783259bClick to view benchmark
|
Description
Part of #6068.
This PR adds couple of things:
[proxy]
in its forc.toml. In a way that it is owned by the deployer and the target initially points to implementation contract.Building
text to the all forc build invocations to better inform the user about what forc is doing behind the scenes.How this works
If the user does not have a proxy table in their forc.toml, nothing changes, same old deployment procedure is followed. Only difference is that this PR improves the ux by removing the need of providing the password multiple times.
If the user has a contract with a proxy table but without an address like:
Forc automatically creates a proxy contract based on the reference implementation at SRC14. Sets its target to the implementation contract, whichever contract enabled the proxy and the owner to the deployer (signing account of the transaction).
If the user has a contract with a proxy table and an address specified like:
Forc automatically makes a set target conract call to update the proxy contract's target. Pointing it to the newly deployed impl contract which defines the proxy table.
Generated proxy contracts are stored at
~/.forc/.generated_proxy_contracts/project_name
for housekeeping.