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 1397 titan canyon unify usdc.e usdc bridge into tokamak contracts bedrock src l2 tokamak #108

Conversation

xxeonge
Copy link
Collaborator

@xxeonge xxeonge commented Mar 25, 2024

  1. Unifying USDC.e and L2UsdcBridge contracts.
  2. Add submodules used by the contracts and remapped their paths in foundry.toml
  3. Linked library for FiatTokenV2_2.sol to the predeploy address of SignatureCheck.sol

@theo-learner
Copy link
Collaborator

@xxeonge Thanks a lot. But the Slither CI test is not passing properly because of some error. Did you check it?

Failed to generate IR for L2UsdcBridge.onlyEOA.

@AaronLee22
Copy link
Collaborator

L2UsdcBridge.onlyEOA.

@xxeonge Thanks a lot. But the Slither CI test is not passing properly because of some error. Did you check it?

Failed to generate IR for L2UsdcBridge.onlyEOA.

Let me check this out too!

@nguyenzung
Copy link
Member

nguyenzung commented Mar 26, 2024

Correct me if I am wrong. I think we can extend OptimismMintableERC20 and use StandardBridge for this task.

Actually, I don't know about the background. What is the benefit of this solution?

@rlgns98kr
Copy link
Member

rlgns98kr commented Mar 26, 2024

Correct me if I am wrong. I think we can extend OptimismMintableERC20 and use StandardBridge for this task.

Actually, I don't know about the background. What is the benefit of this solution?

Thank you, Brave. I'm sorry about it.
There is a standard about USDC and USDC Bridge recommended by Circle (https://www.circle.com/blog/bridged-usdc-standard)

So, Zena made this code. And we will include this code as predeploy!

@xxeonge
Copy link
Collaborator Author

xxeonge commented Mar 26, 2024

@xxeonge Thanks a lot. But the Slither CI test is not passing properly because of some error. Did you check it?

Failed to generate IR for L2UsdcBridge.onlyEOA.

Sorry for late reply. I will check. Thank you!!

@xxeonge xxeonge marked this pull request as draft March 28, 2024 05:04
@xxeonge xxeonge assigned xxeonge and unassigned xxeonge Mar 28, 2024
@AaronLee22 AaronLee22 marked this pull request as ready for review April 1, 2024 02:40
@xxeonge
Copy link
Collaborator Author

xxeonge commented Apr 1, 2024

++) The L1UsdcBridge contract was added to deploy the L1Usdcbridge.

  1. remove libraries tag in foundry.toml, I'll remove it first, as it's better suited for predeploy not unifying.
  2. Unifying L1UsdcBridge and proxy contracts.
  3. libraries and universal related to UsdcBridge have been integrated into the canyon repo.
  4. Modified slither-check.yml to resolve slither errors.

@@ -24,6 +26,7 @@ bytecode_hash = 'none'
build_info = true
build_info_path = 'artifacts/build-info'


Copy link
Member

Choose a reason for hiding this comment

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

Thank you!
I think we can remove this empty line!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rlgns98kr
Thank you! i removed empty lines : )

@rlgns98kr
Copy link
Member

@AaronLee22
Thank you very much!
Do you know why the check action is not worked about latest commit?

@xxeonge xxeonge force-pushed the OR-1397-titan-canyon-unify-USDC.e-USDC-bridge-into-tokamak-contracts-bedrock-src-L2-tokamak branch from 31c4111 to f61ff9c Compare April 2, 2024 01:00
@xxeonge xxeonge self-assigned this Apr 2, 2024
@AaronLee22
Copy link
Collaborator

@AaronLee22 Thank you very much! Do you know why the check action is not worked about latest commit?

Sure, I attempted to resolve this error by following both recommendations from the Slither team, specifically by removing the 'ignore-compile: true' option, yet the issue persisted. The Slither team is currently addressing a known problem where certain libraries or aliases within smart contracts cannot be retrieved. Consequently, I was compelled to manually exclude the specific smart contract experiencing the 'Failed to generate IR' issue.
Screenshot 2024-04-02 at 10 27 57 AM
Screenshot 2024-03-27 at 10 28 05 AM

@@ -0,0 +1,222 @@
// SPDX-License-Identifier: MIT
Copy link
Member

@rlgns98kr rlgns98kr Apr 2, 2024

Choose a reason for hiding this comment

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

I'm sorry. I missed these files.
How about changing the directory structure? I'm sorry again my fault,,, 😢😢

src
--tokamak-contracts
----USDC
------L1
------L2
------libraries
------universal

Copy link
Collaborator Author

@xxeonge xxeonge Apr 2, 2024

Choose a reason for hiding this comment

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

@rlgns98kr
In L2, is there no need to separate the usdc contract and usdcBridge contract files??
If we have to separate them, what name should we give them? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I'm sorry for my late.
About sub directory, I think yours is greeat! So I think you can keep your seperate structure in the L2!

Copy link
Member

@rlgns98kr rlgns98kr 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, @xxeonge and @AaronLee22!
Congratulations for your first contribution! 👍👍

@xxeonge xxeonge force-pushed the OR-1397-titan-canyon-unify-USDC.e-USDC-bridge-into-tokamak-contracts-bedrock-src-L2-tokamak branch from 3cbb4c3 to c0f9851 Compare April 3, 2024 06:58
@xxeonge xxeonge merged commit cefd452 into main Apr 3, 2024
5 checks passed
* validated in the constructor.
*/
bytes32
private constant IMPLEMENTATION_SLOT = 0x7050c9e0f4ca769c69bd3a8ef740bc37934f8e2c036e5a723fd8ee048ed3f8c3;
Copy link
Member

Choose a reason for hiding this comment

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

@xxeonge It is the impl slot for usdc

* validated in the constructor.
*/
bytes32
private constant ADMIN_SLOT = 0x10d6a54a4754c8869d6886b5f5d7fbfa5b4522237ea5c60d11bc4e7a1ff9390b;
Copy link
Member

Choose a reason for hiding this comment

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

It is admin slot for usdc

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.

5 participants