-
Notifications
You must be signed in to change notification settings - Fork 744
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
Contracts expose pallet-xcm #1248
Changes from 40 commits
251e8cd
3b1cc37
22430a3
c6e1f49
bf66839
a348bd9
b2a9f4e
d20d6ca
8388585
4bae246
0675899
2049b57
be04741
b7875ad
c312b98
5983dbd
7484d32
b40f92c
c3f7d40
a08e41b
376a6b3
5b883d6
967616c
d2b5ba3
d7fe1bc
c15adab
e15c7eb
971b167
3dba8d1
be1a46d
a4d706c
b15ea88
b938df4
fcc37f5
9cc990f
7ff220b
bdf0ae7
14e1514
e9d2cdf
c9f8f35
151175b
43f38d2
4583e03
ceb3fc8
db7c479
cd8281e
c004909
526abbd
0a98a89
1d1f40a
e01cb2c
2c7735b
4e3de86
e52f85f
d78ec47
435de45
c4d87ff
4363e6d
ac39a50
0bf6e1a
ddf485a
975ef56
448c9d4
132e203
7bf716d
6a7a8bd
8c1c118
06a5c9a
b47f660
06cf121
0949455
14fed0e
e694941
72f15c1
c8f39e7
54e2803
ce637dc
582467d
79535cb
0a02451
262e17c
9413874
18c6bb4
b9b2ccc
198d692
e3ce2e0
6342962
a2fde63
0ae8763
034150c
7848e2f
b7155b6
34329e5
07439dd
b37d169
de8f1a9
7cd84a2
8ac21da
7ee5d08
e13b51b
2359fa0
41215d3
e515180
8b0f8ac
3526658
784f8df
0b5517a
7b3ad3d
0b4c840
80e0c53
59d67a0
9a9d572
3adde14
c70aca5
5d63636
57b7ca5
10f22fb
162ad54
3a4a251
dab8314
e3161c9
4128aa6
f0df85b
b48a8f9
7c72e24
b34e245
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,20 +48,34 @@ sp-io = { path = "../../primitives/io", default-features = false} | |
sp-runtime = { path = "../../primitives/runtime", default-features = false} | ||
sp-std = { path = "../../primitives/std", default-features = false} | ||
|
||
xcm = { package = "staging-xcm", path = "../../../polkadot/xcm", default-features = false} | ||
xcm-executor = { package = "staging-xcm-executor", path = "../../../polkadot/xcm/xcm-executor", default-features = false} | ||
pallet-xcm = { path = "../../../polkadot/xcm/pallet-xcm", default-features = false} | ||
|
||
[dev-dependencies] | ||
array-bytes = "6.1" | ||
assert_matches = "1" | ||
env_logger = "0.9" | ||
pretty_assertions = "1" | ||
wat = "1" | ||
|
||
# Polkadot Dependencies | ||
xcm-simulator = {path = "../../../polkadot/xcm/xcm-simulator"} | ||
xcm-builder = {package = "staging-xcm-builder", path = "../../../polkadot/xcm/xcm-builder"} | ||
polkadot-runtime-parachains = {path = "../../../polkadot/runtime/parachains"} | ||
polkadot-parachain-primitives = { path = "../../../polkadot/parachain" } | ||
polkadot-primitives = { path = "../../../polkadot/primitives" } | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is wrong: see #1318 Is it possibly to remove any dependency outside Substrate here? If not, why? Unless it is now allowed to commingle any dependency in the SDK, that would erode the notion of any separation of the three previously (intentionally) separate repos. I hope that isn't the case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These dependencies are used to test the pallet in a mocked network environment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not my call, but testing relying on things downstream for Substrate seems wrong to me. I don't see other libraries calling their consumers to test their features... maybe this is actually idiomatic and OK? 🤷 Also might make you build a lot more stuff to get the tests run, but that is just a hunch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree with the opinion that this graph should be enforced. Big point of the monorepo was that which crates are in polkadot vs. cumulus vs. substrate is an arbitrary and ultimately useless distinction. Fact of the matter is that we need those crates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nukemandan all these deps apart from
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the note @pgherveou - that is good news that things are moving away from any |
||
# Substrate Dependencies | ||
pallet-balances = { path = "../balances" } | ||
pallet-timestamp = { path = "../timestamp" } | ||
pallet-message-queue = { path = "../message-queue" } | ||
pallet-insecure-randomness-collective-flip = { path = "../insecure-randomness-collective-flip" } | ||
pallet-utility = { path = "../utility" } | ||
pallet-assets = { path = "../assets" } | ||
pallet-proxy = { path = "../proxy" } | ||
sp-keystore = { path = "../../primitives/keystore" } | ||
sp-tracing = { path = "../../primitives/tracing" } | ||
|
||
[features] | ||
default = [ "std" ] | ||
|
@@ -79,6 +93,7 @@ std = [ | |
"pallet-proxy/std", | ||
"pallet-timestamp/std", | ||
"pallet-utility/std", | ||
"pallet-xcm/std", | ||
"rand/std", | ||
"scale-info/std", | ||
"serde", | ||
|
@@ -90,27 +105,41 @@ std = [ | |
"sp-std/std", | ||
"wasm-instrument/std", | ||
"wasmi/std", | ||
"xcm-executor/std", | ||
"xcm/std", | ||
] | ||
runtime-benchmarks = [ | ||
"frame-benchmarking/runtime-benchmarks", | ||
"frame-support/runtime-benchmarks", | ||
"frame-system/runtime-benchmarks", | ||
"pallet-assets/runtime-benchmarks", | ||
"pallet-balances/runtime-benchmarks", | ||
"pallet-message-queue/runtime-benchmarks", | ||
"pallet-proxy/runtime-benchmarks", | ||
"pallet-timestamp/runtime-benchmarks", | ||
"pallet-utility/runtime-benchmarks", | ||
"pallet-xcm/runtime-benchmarks", | ||
"polkadot-parachain-primitives/runtime-benchmarks", | ||
"polkadot-primitives/runtime-benchmarks", | ||
"polkadot-runtime-parachains/runtime-benchmarks", | ||
"rand", | ||
"rand_pcg", | ||
"sp-runtime/runtime-benchmarks", | ||
"wasm-instrument", | ||
"xcm-builder/runtime-benchmarks", | ||
"xcm-executor/runtime-benchmarks", | ||
] | ||
try-runtime = [ | ||
"frame-support/try-runtime", | ||
"frame-system/try-runtime", | ||
"pallet-assets/try-runtime", | ||
"pallet-balances/try-runtime", | ||
"pallet-insecure-randomness-collective-flip/try-runtime", | ||
"pallet-message-queue/try-runtime", | ||
"pallet-proxy/try-runtime", | ||
"pallet-timestamp/try-runtime", | ||
"pallet-utility/try-runtime", | ||
"pallet-xcm/try-runtime", | ||
"polkadot-runtime-parachains/try-runtime", | ||
"sp-runtime/try-runtime", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
;; This passes its input to `seal_xcm_execute` and returns the return value to its caller. | ||
(module | ||
(import "seal0" "xcm_execute" (func $xcm_execute (param i32 i32) (result i32))) | ||
(import "seal0" "seal_input" (func $seal_input (param i32 i32))) | ||
(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32))) | ||
(import "env" "memory" (memory 1 1)) | ||
|
||
;; 0x1000 = 4k in little endian | ||
;; Size of input buffer | ||
(data (i32.const 0) "\00\10") | ||
|
||
(func (export "call") | ||
;; Receive the encoded call | ||
(call $seal_input | ||
(i32.const 4) ;; Pointer to the input buffer | ||
(i32.const 0) ;; Size of the length buffer | ||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
;; Input data layout. | ||
;; [0..4) - size of the call | ||
;; [4..) - xcm message | ||
|
||
;; Just use the call passed as input and store result to memory | ||
(i32.store (i32.const 0) | ||
(call $xcm_execute | ||
(i32.const 4) ;; Pointer where the xcm msg is stored | ||
(i32.load (i32.const 0)) ;; Size of the xcm msg | ||
) | ||
) | ||
(call $seal_return | ||
(i32.const 0) ;; flags | ||
(i32.const 0) ;; returned value | ||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(i32.const 4) ;; length of returned value | ||
) | ||
) | ||
|
||
(func (export "deploy")) | ||
) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||
;; This passes its input to `seal_xcm_query` and returns the return value to its caller. | ||||||
(module | ||||||
(import "seal0" "xcm_query" (func $xcm_query (param i32 i32 i32) (result i32))) | ||||||
(import "seal0" "seal_input" (func $seal_input (param i32 i32))) | ||||||
(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32))) | ||||||
(import "env" "memory" (memory 1 1)) | ||||||
|
||||||
;; 0x1000 = 4k in little endian | ||||||
;; size of input buffer | ||||||
(data (i32.const 0) "\00\10") | ||||||
|
||||||
(func (export "call") | ||||||
;; Receive the encoded call | ||||||
(call $seal_input | ||||||
(i32.const 4) ;; Pointer to the input buffer | ||||||
(i32.const 0) ;; Size of the length buffer | ||||||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
;; Input data layout. | ||||||
;; [0..4) - size of the call | ||||||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
;; [4..12) - timeout | ||||||
;; [12..49) - match_querier | ||||||
;; [8..10) response | ||||||
|
||||||
;; Just use the call passed as input and store result to memory | ||||||
(i32.store (i32.const 0) | ||||||
(call $xcm_query | ||||||
(i32.const 4) ;; Pointer where the timeout is stored | ||||||
(i32.const 12) ;; Pointer where the match_querier is stored | ||||||
(i32.const 49) ;; ... | ||||||
) | ||||||
) | ||||||
(call $seal_return | ||||||
(i32.const 0) ;; flags | ||||||
(i32.const 49) ;; returned value | ||||||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
(i32.const 8) ;; length of returned value | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
) | ||||||
) | ||||||
|
||||||
(func (export "deploy")) | ||||||
) | ||||||
|
||||||
|
||||||
|
||||||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
;; This passes its input to `seal_xcm_send` and returns the return value to its caller. | ||
(module | ||
(import "seal0" "xcm_send" (func $xcm_send (param i32 i32 i32) (result i32))) | ||
(import "seal0" "seal_input" (func $seal_input (param i32 i32))) | ||
(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32))) | ||
(import "env" "memory" (memory 1 1)) | ||
|
||
;; 0x1000 = 4k in little endian | ||
;; size of input buffer | ||
(data (i32.const 0) "\00\10") | ||
|
||
(func (export "call") | ||
;; Receive the encoded call | ||
(call $seal_input | ||
(i32.const 4) ;; Pointer to the input buffer | ||
(i32.const 0) ;; Size of the length buffer | ||
) | ||
;; Input data layout. | ||
;; [0..4) - size of the call | ||
;; [4..7) - dest | ||
;; [7..) - xcm message | ||
|
||
;; Just use the call passed as input and store result to memory | ||
(i32.store (i32.const 0) | ||
(call $xcm_send | ||
(i32.const 4) ;; Pointer where the dest is stored | ||
(i32.const 7) ;; Pointer where the xcm msg is stored | ||
(i32.sub ;; Size of the xcm msg | ||
(i32.load (i32.const 0)) | ||
(i32.const 3) | ||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
) | ||
) | ||
(call $seal_return | ||
(i32.const 0) ;; flags | ||
(i32.const 0) ;; returned value | ||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(i32.const 4) ;; length of returned value | ||
) | ||
) | ||
|
||
(func (export "deploy")) | ||
) | ||
|
||
|
||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
;; This passes its input to `seal_xcm_take_response` and returns the return value to its caller. | ||
(module | ||
(import "seal0" "xcm_take_response" (func $xcm_take_response (param i32 i32) (result i32))) | ||
(import "seal0" "seal_input" (func $seal_input (param i32 i32))) | ||
(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32))) | ||
(import "env" "memory" (memory 1 1)) | ||
|
||
;; 0x1000 = 4k in little endian | ||
;; size of input buffer | ||
(data (i32.const 0) "\00\10") | ||
|
||
(func (export "call") | ||
;; Receive the encoded call | ||
(call $seal_input | ||
(i32.const 4) ;; Pointer to the input buffer | ||
(i32.const 0) ;; Size of the length buffer | ||
) | ||
;; Input data layout. | ||
;; [0..4) - size of the call | ||
;; [4..12) - query_id | ||
;; [12..) - response | ||
|
||
;; Just use the call passed as input and store result to memory | ||
(i32.store (i32.const 0) | ||
(call $xcm_take_response | ||
(i32.const 4) ;; Pointer where the query_id is stored | ||
(i32.const 12) ;; Pointer where the response is stored | ||
) | ||
) | ||
|
||
(call $seal_return | ||
(i32.const 0) ;; flags | ||
(i32.const 12) ;; returned value | ||
(i32.const 112) ;; length of returned value | ||
) | ||
) | ||
|
||
(func (export "deploy")) | ||
) | ||
|
||
|
||
|
||
|
||
pgherveou marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these crates are going to move into their own repo (cc @KiChjang ), so that should partially address your concern.
I don't see how we could bring xcm features to SC without bringing in these dependencies though... 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, makes sense.
Not my call, but I would strive to have things in the dep graph be correct before landing this PR. Else the slippery slope to a janky cluster of deps crossing boundaries will likely happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree you don't necessarily want to use xcm when using pallet-contracts, you might not even have access to it
Ideally we want contracts, at the pallet level, to allow xcm but not enforce knowing about it
It's good that you can disable the capability, but still relying on this dependencies sounds too much when I don't want to deal with xcm as a solochain developer for example
The only way to take this out of the dependencies would be to have a different crate, something like "xcm-powered-contracts", which is ugly. The other thing we could do is feature gate this, so at least some things are not even compiled if you don't want XCM. However, dependencies can't be defined per feature 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reasoning is that even solo chain might eventually want to use xcm, to communicate messages over bridges for example. That's why we want xcm to be part of the config, rather than using other type of indirection like dispatching runtime call or building ad-hoc SC chain extension, which has been how team have interfaced with XCM so far.
We could feature gate everything but I think this is not necessary as the compiler should just strip pallet_xcm code if you use the default
()
noop trait.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes XCM is not specific to being a parachain. It is a language to express intent. It will also be used for contract <-> runtime communication. In an ideal world it would also be used inside a transaction instead of the unstable encoded
Call
.Hence there is no reason to jump though all these hoops and add all the complexity just to make it optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I initially I used tight coupling , but I think having an Xcm configuration trait provide a few advantage:
T::Xcm::some_xcm_foo(..)
rather than<T as pallet_xcm::Config>::xcm_foo(..)
T::
by<T as Config>
to get the pallet to compilepallet_xcm::Config
straight away.Happy to update if you feel strongly that we should use tight coupling on the pallet.