SNO-425: For parachain -> eth messages, sync by parachain id instead of account id #806

doubledup · 2023-04-14T11:20:09Z

This also makes the parachain relayer sync messages for a single configured parachain id instead of multiple.

Issue
Companion PR in Cumulus fork (will bump the submodule here once the companion's merged)

core/packages/test/config/parachain-relay.json

parachain/pallets/outbound-queue/src/lib.rs

vgeddes · 2023-04-14T12:24:09Z

The other is an unresolved import for snowbridge_router_primitives::ConvertTokenAddress, which doesn't sound related to this change.

This should be fixed in latest master.

doubledup · 2023-04-14T14:39:12Z

parachain/pallets/outbound-queue/src/lib.rs

 	fn into(self) -> Token {
 		Token::Tuple(vec![
-			Token::Bytes(self.parachain_id.encode()),
+			Token::Uint(u32::from(self.origin).into()),


Had to add an explicit conversion here because Uint has a From<u32> but no From<polkadot_parachain::primitives::Id>

SCALE uses Little Endian.

Creating a non-nil slice will make `slice != nil` checks return true when there's nothing in the slice anyway. Also nil slices are preferred: https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

yrong

LGTM!

relayer/relays/parachain/scanner.go

parachain/pallets/outbound-queue/src/test.rs

This is now done in the loop.

The inbound channel contract uses OpenZeppelin's single MerkleProof, which sorts hashes before concatenation instead of including the order in the proof.

This method no longer contains "basic": https://github.com/Snowfork/snowbridge/blob/2922b973af9d4373a4c7a149988919c35fed9d62/parachain/pallets/outbound-queue/rpc/src/lib.rs#L31

alistair-singh · 2023-04-18T10:57:24Z

relayer/relays/parachain/config.go

 	Contracts SourceContractsConfig  `mapstructure:"contracts"`
 	// Block number when Beefy was activated
 	BeefyActivationBlock uint64 `mapstructure:"beefy-activation-block"`
+	LaneID               uint32 `mapstructure:"lane-id"`


A LaneId is a ParaId. I am not sure if we should use different terms for the same thing. In the outbound queue, we use ParaId and make no mention of Lanes. In the Rococo-Wococo bridge, they use a concept of a lane as well and it seems to be an arbitrary id that routes to but is not a ParaId.

Right, that could be confusing. This was based on the execution relayer config, so I'd rather do it in a follow-up PR so we don't change the execution relayer here. LaneIDs only exist in our relayers, in the queues & messages we call it origin but paraID sounds fine for the relayers.

@vgeddes Are you happy with ParaID in the relayers?

Semi-related: I noticed a bunch of sourceID references and changed them to laneID for consistency.

doubledup added 3 commits April 13, 2023 20:58

Make parachain relayer relay a single sourceID

a095d40

Switch to parachainID in outbound queue

62b73d0

Set up sourceID config for parachain relayer

7610dc9

doubledup requested a review from vgeddes April 14, 2023 11:20

doubledup self-assigned this Apr 14, 2023

doubledup marked this pull request as draft April 14, 2023 11:20

vgeddes reviewed Apr 14, 2023

View reviewed changes

doubledup added 5 commits April 14, 2023 15:25

s/SourceID/LaneID/g

13e1ced

s/parachain_id/origin/g

8ad9875

Add handler to submit() and Message

0f43390

Serialize origin as a number

66cdefb

Merge branch 'main' into david/sno-425

4e2f2ad

doubledup changed the title ~~David/SNO-425: For parachain -> eth messages, sync by parachain id instead of account id~~ SNO-425: For parachain -> eth messages, sync by parachain id instead of account id Apr 14, 2023

doubledup mentioned this pull request Apr 14, 2023

SNO-425: For parachain -> eth messages, sync by parachain id instead of account id Snowfork/cumulus#18

Merged

Fix message fields in benchmark

52fd689

doubledup commented Apr 14, 2023

View reviewed changes

doubledup added 8 commits April 14, 2023 16:44

Make Bob's id different from Alice's

40bc9c4

Remove unused test imports

b58bb24

Remove endianness TODO

c127ca7

SCALE uses Little Endian.

Leave return value as nil slice

3d7176c

Creating a non-nil slice will make `slice != nil` checks return true when there's nothing in the slice anyway. Also nil slices are preferred: https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

Replace sort with reverse

22a095a

Clean up block scanning loop

530454b

Tweak scanForOutboundQueueProofs

6dc3628

Handle partially relayed message blocks

ff458c2

doubledup force-pushed the david/sno-425 branch from 23b26ce to ff458c2 Compare April 17, 2023 13:00

doubledup marked this pull request as ready for review April 17, 2023 13:25

doubledup requested review from alistair-singh and yrong April 17, 2023 13:59

Set lane-id for parachain relayer

194edfd

Merge branch 'main' into david/sno-425

b39f554

yrong approved these changes Apr 18, 2023

View reviewed changes

relayer/relays/parachain/scanner.go Show resolved Hide resolved

relayer/relays/parachain/scanner.go Show resolved Hide resolved

parachain/pallets/outbound-queue/src/test.rs Outdated Show resolved Hide resolved

doubledup added 6 commits April 18, 2023 10:45

Remove extra block number decrement

5663790

This is now done in the loop.

Error when proof & digest item hashes are mismatched

850551f

Rename alice & bob parachain ids

38a16ca

Replace digest item commitment with hash

520e9de

Remove unused hashSides field

9f9ba92

The inbound channel contract uses OpenZeppelin's single MerkleProof, which sorts hashes before concatenation instead of including the order in the proof.

Fix proof RPC method name

651093c

This method no longer contains "basic": https://github.com/Snowfork/snowbridge/blob/2922b973af9d4373a4c7a149988919c35fed9d62/parachain/pallets/outbound-queue/rpc/src/lib.rs#L31

alistair-singh reviewed Apr 18, 2023

View reviewed changes

doubledup added 2 commits April 18, 2023 14:07

s/sourceID/laneID/g

68cf78a

Finish renaming test paraID

69300fa

alistair-singh approved these changes Apr 18, 2023

View reviewed changes

doubledup added a commit to Snowfork/cumulus that referenced this pull request Apr 18, 2023

Companion for Snowfork/snowbridge#806 (#18)

179759d

doubledup added 4 commits April 18, 2023 17:59

Update Cumulus submodule

c65fd9d

Merge branch 'main' into david/sno-425

ee8e7f1

Remove rogue hashSides reference

5bf7ef0

Fix lane-id reference for parachain relayer config

b6bd532

doubledup merged commit 36c3279 into main Apr 18, 2023

doubledup deleted the david/sno-425 branch April 18, 2023 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SNO-425: For parachain -> eth messages, sync by parachain id instead of account id #806

SNO-425: For parachain -> eth messages, sync by parachain id instead of account id #806

Uh oh!

doubledup commented Apr 14, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgeddes commented Apr 14, 2023

Uh oh!

doubledup Apr 14, 2023

Uh oh!

yrong left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alistair-singh Apr 18, 2023

Uh oh!

doubledup Apr 18, 2023

Uh oh!

doubledup Apr 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SNO-425: For parachain -> eth messages, sync by parachain id instead of account id #806

SNO-425: For parachain -> eth messages, sync by parachain id instead of account id #806

Uh oh!

Conversation

doubledup commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgeddes commented Apr 14, 2023

Uh oh!

doubledup Apr 14, 2023

Choose a reason for hiding this comment

Uh oh!

yrong left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alistair-singh Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

doubledup Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

doubledup Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

doubledup commented Apr 14, 2023 •

edited

Loading