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

Aptos OFT wire #48

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

dhruvinparikh
Copy link
Contributor

No description provided.

@pegahcarter
Copy link
Collaborator

This is great. LMK when the PR is at a point ready for review.

delegate: string,
dvnHorizen?: string,
dvnL0?: string,
dvn1?: string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

dvn1 and dvn2 are accidental and can be changed to dvnHorizen / dvnL0.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll do this in a separate PR.

@pegahcarter
Copy link
Collaborator

pegahcarter commented Feb 20, 2025

Are you going to need the addresses from L0Constants.sol to import as peers? If so, should L0Constants.sol instead be a json that is readable by typescript?

I'm thinking if we go with a json, it would look something like this:

{
    "30101": {
        "fxs": "0x...",
        "frxUsd": "0x..."
    },
    "30255": {}
}

Where the keys are the EID. Unless you think the key should be chainid? Or should this be within L0Config.json? I'm flexible here.

@dhruvinparikh
Copy link
Contributor Author

Are you going to need the addresses from L0Constants.sol to import as peers? If so, should L0Constants.sol instead be a json that is readable by typescript?

I'm thinking if we go with a json, it would look something like this:

{
    "30101": {
        "fxs": "0x...",
        "frxUsd": "0x..."
    },
    "30255": {}
}

Where the keys are the EID. Unless you think the key should be chainid? Or should this be within L0Config.json? I'm flexible here.

For wiring txs on EVMs, it needs to be done from multisig so will Solidity scripts will be use L0Constants.sol as well as L0Config.json

@pegahcarter
Copy link
Collaborator

We could refactor the EVM deploy script so that it loads the json file addresses rather than from the solidity file. I'm just thinking of what constants will be re-used across VMs.

@@ -100,7 +100,7 @@ function getConnectionConfig(lzConfig: lzConfigType[], sourceContract: OmniPoint
receiveConfig: {
ulnConfig: {
confirmations: BigInt(5),
requiredDVNs: [sourceOFTConfig.dvnHorizen, sourceOFTConfig.dvnL0],
requiredDVNs: [sourceOFTConfig.dvnHorizen ?? "", sourceOFTConfig.dvnL0 ?? ""],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this be an issue if there's an array of [0x123, ""] as it's length 2?

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