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

Permissionless Adapters #20

Merged
merged 26 commits into from
Jun 17, 2024
Merged

Permissionless Adapters #20

merged 26 commits into from
Jun 17, 2024

Conversation

jboetticher
Copy link
Collaborator

@jboetticher jboetticher commented May 13, 2024

  • Removes custom adapters array & GMP ID array
  • Replaces adapter parameters with a single address array that accepts address(0) to address(248) as reserved for glacis (reservation domain subject to change, potentially we change it to 0xFFFF so that we don't limit ourselves)
  • Added an array of adapters to the receive message parameters
  • Finishes SimpleTokenAdapter for developers who don't want to use our token adapter

@gusjavaz Would be helpful if you looked this over when you get the chance (no rush as Halborn is looking through the current code still). Don't merge still if you do approve, as I'd like the Halborn folks to also take a look at the PR.

Also, coverage works again:

Screenshot 2024-05-13 at 12 16 28 PM

@jboetticher jboetticher mentioned this pull request May 14, 2024
/// GlacisTokenMediator has. Similarly, the retry function has been replaced
/// with a `sendCrossChainRetry`.
/// Developers using this must ensure that their token has the same address on
/// each chain.
contract SimpleTokenMediator is
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: What about XERC20Mediator as name? Since it is using XERC20 pure standard without the extension perhaps it is more informative of the overall functionality of the contract

contracts/mediators/SimpleTokenMediator.sol Outdated Show resolved Hide resolved
test/LocalTestSetup.sol Show resolved Hide resolved
@@ -10,7 +10,6 @@ import {AddressBytes32} from "../libraries/AddressBytes32.sol";

error GlacisRouter__GMPNotSupported(); //0xed2e8008
Copy link
Contributor

@gusjavaz gusjavaz May 16, 2024

Choose a reason for hiding this comment

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

Could we remove this error signatures?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's required if one of our reserved GMPs isn't used yet

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean what's after "//0x", it breaks the error table generating script

contracts/routers/GlacisAbstractRouter.sol Show resolved Hide resolved
@@ -2,7 +2,7 @@

pragma solidity 0.8.18;

import "@openzeppelin/contracts/access/Ownable.sol";
import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator Author

@jboetticher jboetticher left a comment

Choose a reason for hiding this comment

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

There's actually a major issue with token mediators now. EOAs can't always accept now because the fromGmpId could be a custom adapter.

        // If the destination smart contract is an EOA, then we allow it.
        address toAddress = to.toAddress();
        if (toAddress.code.length == 0) {
            return true;
        }

@jboetticher
Copy link
Collaborator Author

Should also leave uint8 if necessary and standardize to a higher bound (based off of GLACIS_RESERVED_IDS)

@jboetticher jboetticher merged commit e1c7fef into main Jun 17, 2024
1 check passed
@jboetticher jboetticher deleted the permissionless-adapters branch June 17, 2024 17:46
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.

None yet

2 participants