Skip to content
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

Or 1535 thanos cannon update for predeploy contracts #132

Merged
merged 6 commits into from
Apr 17, 2024

Conversation

nguyenzung
Copy link
Member

@nguyenzung nguyenzung commented Apr 17, 2024

  • Update for predeploy contracts

Test: you can replace "http://localhost:9545" to your L2_RPC

  • USDC:

decimal(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000778","data":"0x313ce567"}, "latest"],"id":1}' http://localhost:9545

admin(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000778","data":"0xf851a440"}, "latest"],"id":1}' http://localhost:9545

implementation(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000778","data":"0x5c60da1b"}, "latest"],"id":1}' http://localhost:9545

  • Bridge:

admin(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000775","data":"0xf851a440"}, "latest"],"id":1}' http://localhost:9545

implementation(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000775","data":"0x5c60da1b"}, "latest"],"id":1}' http://localhost:9545

messenger(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000775","data":"0x3cb747bf"}, "latest"],"id":1}' http://localhost:9545

l1Usdc(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000775","data":"0x56c3b587"}, "latest"],"id":1}' http://localhost:9545

l2Usdc(): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000775","data":"0xa1b4bc04"}, "latest"],"id":1}' http://localhost:9545

  • MasterMinter:

getWorker(address): curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x4200000000000000000000000000000000000777","data":"0xc011b1c30000000000000000000000004200000000000000000000000000000000000775"}, "latest"],"id":1}' http://localhost:9545

Thank you very much!

@nguyenzung nguyenzung self-assigned this Apr 17, 2024
@xxeonge
Copy link
Collaborator

xxeonge commented Apr 17, 2024

Hello! @nguyenzung

In my opinion, since the deployment address of SignatureChecker is included during the deployment of FiatTokenV2_2.sol(it can be checked in foundry.toml), it seems we should deploy SignatureChecker as well.
Additionally, when I was modifying the predeploy deployment code, Justin from the smart contract team mentioned that even though it's a library, if it contains public or external functions, it should be deployed!

Thank you😊

@steven94kr
Copy link
Member

@nguyenzung
Oh, I missed something else,,,,(Is it failed to make devnet-up as well to you?,,,)
I will check it after lunch,,, 🥹🙇🏼‍♂️

@nguyenzung
Copy link
Member Author

Hello! @nguyenzung

In my opinion, since the deployment address of SignatureChecker is included during the deployment of FiatTokenV2_2.sol(it can be checked in foundry.toml), it seems we should deploy SignatureChecker as well. Additionally, when I was modifying the predeploy deployment code, Justin from the smart contract team mentioned that even though it's a library, if it contains public or external functions, it should be deployed!

Thank you😊

Thank you!

Let me roll it back. Anw, I am curious about why we should deploy a lib if it contains public or external functions

@nguyenzung
Copy link
Member Author

@nguyenzung Oh, I missed something else,,,,(Is it failed to make devnet-up as well to you?,,,) I will check it after lunch,,, 🥹🙇🏼‍♂️

Thank you very much, Steven

I can start devnet, but when I looked at the PR about USDC, I found that there are things need to be checked more. I am doing it in this PR

@xxeonge
Copy link
Collaborator

xxeonge commented Apr 17, 2024

https://eip2535diamonds.substack.com/p/the-difference-between-solidity-libraries

I will send you a link for reference !! I don’t know if there will be an answer..!🥹 @nguyenzung

@@ -68,6 +69,13 @@ func BuildL2Genesis(config *DeployConfig, l1StartBlock *types.Block) (*core.Gene
db.CreateAccount(codeAddr)
db.SetState(addr, ImplementationSlot, eth.AddressAsLeftPaddedHash(codeAddr))
log.Info("Set proxy", "name", name, "address", addr, "implementation", codeAddr)

Copy link
Member

@steven94kr steven94kr Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've catched this points! You are so great. Thank you very much. 🙇🏼‍♂️

FiatTokenV2 use the

How about adding the variable for slot in here ?

ImplementationSlot = common.HexToHash("0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc")

ImplementationSlotForZepplin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much, Steven

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me create a variable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated!

case predeploys.FiatTokenV2_2Addr:
db.SetCode(addr, fiatTokenProxyBytecode)
db.SetState(addr, common.HexToHash("0x10d6a54a4754c8869d6886b5f5d7fbfa5b4522237ea5c60d11bc4e7a1ff9390b"), eth.AddressAsLeftPaddedHash(proxyAdminAddr))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can add variable as well!

AdminSlotForZepplin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated

storage["MasterMinter"] = state.StorageValues{
"_owner": config.MasterMinterOwner,
"controllers": map[any]any{
"_controller": predeploys.L2UsdcBridgeAddr,
"_worker": predeploys.L2UsdcBridgeAddr,
predeploys.L2UsdcBridgeAddr: predeploys.L2UsdcBridgeAddr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Thank you very much, Brave,,,🙇🏼‍♂️😍🥰

@steven94kr
Copy link
Member

Oh, I'm sorry. I have a question! 🙇🏼‍♂️
@xxeonge @nguyenzung
If I run the make bindings-build, There is some update of usdcBridge's binding files.
image
Do you know why? And should we update the files? 🙇🏼‍♂️

@nguyenzung
Copy link
Member Author

Oh, I'm sorry. I have a question! 🙇🏼‍♂️ @xxeonge @nguyenzung If I run the make bindings-build, There is some update of usdcBridge's binding files. image Do you know why? And should we update the files? 🙇🏼‍♂️

I think we need to view in assembly to verify it. Let me check

@steven94kr
Copy link
Member

Thank you very much,,,🙇🏻‍♂️🥰🫡

@xxeonge
Copy link
Collaborator

xxeonge commented Apr 17, 2024

Thank you so much😊
When I tried it, the file didn't change, so I think I need to check with @nguyenzung

@steven94kr
Copy link
Member

Thank you so much😊 When I tried it, the file didn't change, so I think I need to check with @nguyenzung

If you delete forge-artifacts and build again, I think you can see the changes.
After that, if you still can't see the changes, please contact to me.

@xxeonge
Copy link
Collaborator

xxeonge commented Apr 17, 2024

Thank you so much😊 When I tried it, the file didn't change, so I think I need to check with @nguyenzung

If you delete forge-artifacts and build again, I think you can see the changes. After that, if you still can't see the changes, please contact to me.

I cloned a new main and ran make bindings-build, but the file does not change..! OR-1535-thanos-cannon-update-for-predeploy-contracts as well @rlgns98kr

@steven94kr
Copy link
Member

@xxeonge
Oh, Thank you for your checking. I will check again about the changes,,,🙇🏼‍♂️

@nguyenzung
Copy link
Member Author

nguyenzung commented Apr 17, 2024

Oh, I'm sorry. I have a question! 🙇🏼‍♂️ @xxeonge @nguyenzung If I run the make bindings-build, There is some update of usdcBridge's binding files. image Do you know why? And should we update the files? 🙇🏼‍♂️

I think we need to view in assembly to verify it. Let me check

It seems like the change does not have any meaning.
Screenshot from 2024-04-17 13-44-54
Screenshot from 2024-04-17 13-45-03

Line 0x11EC and 0x11ED are located between 2 STOP instructions, and those line does not change storage slot or does not return anything. Maybe the change related to compile config

@nguyenzung
Copy link
Member Author

https://eip2535diamonds.substack.com/p/the-difference-between-solidity-libraries

I will send you a link for reference !! I don’t know if there will be an answer..!🥹 @nguyenzung

I tested by debugging, but it seems not run as what described in the article.

When we deploy a contract that using lib with external function, there is one transaction. Can we have one transaction for deploying 2 addresses?

Anw, the code was rolled back. I think it is good for now

Copy link
Member

@steven94kr steven94kr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much, Brave! 🥰

@nguyenzung nguyenzung merged commit 8ee6d4f into main Apr 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants