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

Update bridged_USDC_standard.md #441

Merged
merged 3 commits into from
Mar 15, 2024
Merged

Conversation

blincf
Copy link
Contributor

@blincf blincf commented Mar 14, 2024

v2 of the doc. Updates specifically on the contract details and the token roles.

Summary

Added more clarity on contract details and token roles in this v2 of the doc.

Detail

As summary.

Testing

NA

Documentation

That's this PR's main change.


v2 of the doc. Updates specifically on the contract details and the token roles.
Copy link
Contributor

@yvonnezhangc yvonnezhangc left a comment

Choose a reason for hiding this comment

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

Thanks so much for updating the doc! Two minor comments.


The third-party team’s bridge contracts must be upgradeable to add required, specific functionality at a later point in time to support the upgrade steps above, once Circle and the third-party team jointly decide to perform the upgrade.
The third-party team’s bridge contracts play an integral role and require small additions to support the upgrade process.
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: are we intentionally removing source bridge upgradeability as a requirement?
If not, I feel like this is still an important detail to call out. We should considering specifying the defined interface for burnAllLockedUSDC as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The requirement is specified below on burn, but my understanding is that this doc intends to clarify things before they start deploying, and we recommend them to add the burn functionality after we mutually agree. So they aren't missing out this particular function name particularly. The following two lines hinted some on this regard.

function transferUSDCRoles(address owner) external;
```

The function implementation details are left up to the partner, but it must: 1) only be callable by a Circle-owned address and 2) transfer the Implementation Owner role and the ProxyAdmin role to the function caller (if both are assigned to the bridge).
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on how the interface is defined, I believe the ProxyAdmin role will be assigned to the function caller, where as the new owner address should be specified in the function parameter.

function transferUSDCRoles(address owner)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes in some ways we separate these into two function calls (they transfer the proxyAdmin to us first, and they call this function to transfer the owner role to us which we move the remaining roles).
The goal for this doc isn't to specify the upgrade process which often times requires more digging and isn't something we want every team to be ready to do before we agree to convert. What we can do is to make sure that they don't hit the problems when deploying bridged token that makes it impossible for future upgrade. This doc shouldn't describe the upgrade process in that level of detail.
Do you agree @mauriziocircle

update some wording.
@yvonnezhangc yvonnezhangc merged commit 0828084 into circlefin:master Mar 15, 2024
3 checks passed
zhenghui-w pushed a commit to zhenghui-w/stablecoin-evm that referenced this pull request Apr 8, 2024
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.

2 participants