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

chore: fusdc ChainPolicy schema #10679

Merged
merged 1 commit into from
Dec 12, 2024
Merged

chore: fusdc ChainPolicy schema #10679

merged 1 commit into from
Dec 12, 2024

Conversation

0xpatrickdev
Copy link
Member

@0xpatrickdev 0xpatrickdev commented Dec 11, 2024

Description

  • rename nobleContractAddress to attenuatedCttpBridgeAddress as it better captures the functionality in FUSDC.
    • the contract deployed to this address must be an attenuated wrapper around Circle's TokenMessenger that does not contain replaceDepositForBurn . The Oracle Nodes must only report transactions where depositor for DepositForBurn is this value.
  • include jsdoc comments for ChainPolicy type

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

Improves docs via jsdoc annotations

Testing Considerations

None

Upgrade Considerations

None, unreleased code

@0xpatrickdev 0xpatrickdev requested a review from a team as a code owner December 11, 2024 18:45
Copy link

cloudflare-workers-and-pages bot commented Dec 11, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8955990
Status: ✅  Deploy successful!
Preview URL: https://40318615.agoric-sdk.pages.dev
Branch Preview URL: https://pc-chain-policy-schema.agoric-sdk.pages.dev

View logs

@@ -58,11 +58,14 @@ export interface PoolMetrics extends PoolStats {
}

export interface ChainPolicy {
nobleContractAddress: EvmHash;
/** `msg.sender` of DepositAndBurn to TokenMessenger must be an attenuated wrapper contract that does not contain `replaceDepositForBurn` */
Copy link
Member

Choose a reason for hiding this comment

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

👌

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a few words from this JSDoc lifed to the PR description and/or title.

not critical.

confirmations: M.number(),
chainId: M.number(),
},
{ chainType: M.number() },
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -17,10 +17,10 @@ Generated by [AVA](https://avajs.dev).
{
chainPolicies: {
Arbitrum: {
attenuatedCttpBridgeAddress: '0xe298b93ffB5eA1FB628e0C0D55A43aeaC268e347',
Copy link
Contributor

Choose a reason for hiding this comment

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

This address is different than the previous nobleContractAddress, should we update it in multichain-testing/test/fast-usdc/config.ts as well?

Copy link
Member Author

@0xpatrickdev 0xpatrickdev Dec 11, 2024

Choose a reason for hiding this comment

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

Good catch I am basing this on top of master (instead), but I'll make sure to make this change there too

@@ -68,7 +68,8 @@ export const configurations = {
'0x19330d10D9Cc8751218eaf51E8885D058642E08A',
chainId: 42161,
confirmations: 2,
nobleContractAddress: '0x19330d10D9Cc8751218eaf51E8885D058642E08A',
attenuatedCttpBridgeAddress:
'0xe298b93ffB5eA1FB628e0C0D55A43aeaC268e347',
Copy link
Member

Choose a reason for hiding this comment

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

were these properties previously sorted?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -58,11 +58,14 @@ export interface PoolMetrics extends PoolStats {
}

export interface ChainPolicy {
nobleContractAddress: EvmHash;
/** `msg.sender` of DepositAndBurn to TokenMessenger must be an attenuated wrapper contract that does not contain `replaceDepositForBurn` */
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a few words from this JSDoc lifed to the PR description and/or title.

not critical.

@0xpatrickdev 0xpatrickdev force-pushed the pc/fusdc-multichain branch 2 times, most recently from 4ed677f to d99ae45 Compare December 11, 2024 23:23
@0xpatrickdev 0xpatrickdev force-pushed the pc/chain-policy-schema branch from 48171d8 to f046f07 Compare December 11, 2024 23:27
@0xpatrickdev 0xpatrickdev changed the base branch from pc/fusdc-multichain to master December 11, 2024 23:27
Copy link

Base branch is changed to master. Please re-run the integration tests by adding 'force:integration' label.

@0xpatrickdev 0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Dec 11, 2024
- rename `nobleContractAddress` to `attenuatedCttpBridgeAddress`
- include jsdoc comments for `ChainPolicy` type
- removed `chainType` from `ChainPolicy`. unused and should have been an enum. not needed for OCW currently
@0xpatrickdev 0xpatrickdev force-pushed the pc/chain-policy-schema branch from f046f07 to 8955990 Compare December 12, 2024 00:49
@mergify mergify bot merged commit bdf5c17 into master Dec 12, 2024
81 checks passed
@mergify mergify bot deleted the pc/chain-policy-schema branch December 12, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants