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 polkadot dependencies #417

Merged
merged 27 commits into from
Jun 28, 2021
Merged

Update polkadot dependencies #417

merged 27 commits into from
Jun 28, 2021

Conversation

vgeddes
Copy link
Collaborator

@vgeddes vgeddes commented Jun 4, 2021

Updated Polkadot/Cumulus dependencies to polkadot v0.9.4. This also required us to update GSRPC to get metadata V13 changes.

Note that XCM verification & testing is pushed out to a follow-up PR given the scope-creep that has arisen so far.

E2E tests mostly pass. For the tests that fail, there is an unrelated & pre-existing issue in the commitment relayer which results in some commitments getting ignored and are not being passed on to Ethereum.

Changes:

Copy link
Contributor

@philipstanislaus philipstanislaus left a comment

Choose a reason for hiding this comment

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

Cool, few minor comments

parachain/runtime/local/src/lib.rs Show resolved Hide resolved
parachain/runtime/local/src/lib.rs Outdated Show resolved Hide resolved
parachain/runtime/snowbridge/src/lib.rs Show resolved Hide resolved
@vgeddes vgeddes marked this pull request as ready for review June 23, 2021 11:27
@vgeddes vgeddes requested review from Rizziepit and musnit June 23, 2021 11:28
Copy link
Contributor

@philipstanislaus philipstanislaus left a comment

Choose a reason for hiding this comment

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

Nice, just few questions 💪

Comment on lines +333 to +334
// We don't track any teleports.
()
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reasoning behind that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By design, we don't support teleportation. Only reserve-backed transfers, which doesn't require trust in the other chain.

type IsReserve = NativeAsset;
type IsTeleporter = ();
type IsTeleporter = NativeAsset; // <- should be enough to allow teleportation of ROC
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we support teleportation though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, will fix!

type Barrier = Barrier;
type Weigher = FixedWeightBounds<UnitWeightCost, Call>;
type Trader = UsingComponents<IdentityFee<Balance>, RococoLocation, AccountId, Balances, ()>;
type ResponseHandler = (); // Don't handle responses for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No idea. 😅

XCM documentation is sparse and a lot of the new XCM code I copy-pasted from the Cumulus parachains at https://github.com/paritytech/cumulus/tree/master/polkadot-parachains (statemint, rococo examples, etc)

@vgeddes vgeddes merged commit 4ca7a6b into main Jun 28, 2021
@vgeddes vgeddes deleted the update-cumulus branch June 28, 2021 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants