-
Notifications
You must be signed in to change notification settings - Fork 443
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
Add xcm integration #1912
Add xcm integration #1912
Conversation
adef8b3
to
2baefa4
Compare
3813349
to
0aa22e7
Compare
Co-authored-by: Michael Müller <michi@parity.io>
Co-authored-by: Michael Müller <michi@parity.io>
¶chain_account_sovereign_account_id(1, account_id) | ||
), | ||
vec![BalanceLock { | ||
id: *b"py/xcmlk", |
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.
Please add a comment that describes why this literal is used here and how to determine which ones to use in different scenarios.
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.
This comment was marked resolved, but I don't see a comment in the Files Changed. I'm unresolving hence.
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.
my bad, mis-clicked it, will add comments here
|
||
// Double encoding the message as the host fn expects an encoded message. | ||
let enc_msg = scope.take_encoded(&scale::Encode::encode(msg)); | ||
#[allow(deprecated)] |
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.
Why is the function already marked deprecated?
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.
unstable host api (which is still the case for this one as of this release of polkadot-sdk) are marked as deprecated so that they can trigger a warning if you try to use them without specifying allow(deprecated)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1912 +/- ##
==========================================
+ Coverage 60.56% 61.30% +0.74%
==========================================
Files 140 139 -1
Lines 5779 5709 -70
Branches 2448 2421 -27
==========================================
Hits 3500 3500
+ Misses 2279 2209 -70 ☔ View full report in Codecov by Sentry. |
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑These are the results when building the
Link to the run | Last update: Fri Apr 26 18:59:01 CEST 2024 |
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.
Looks good, happy to finally have xcm in ink!
🚀
Just the thing with the encoding of the message and I think we are good to go
@@ -622,7 +622,7 @@ jobs: | |||
# - custom_allocator | |||
# Pulls in sp-std which needlessly requires atomic pointers (TODO: Fix sp-std and enable this example) | |||
# - call-runtime | |||
scripts/for_all_contracts_exec.sh --path integration-tests --ignore public/custom-allocator --ignore public/call-runtime \ | |||
scripts/for_all_contracts_exec.sh --path integration-tests --ignore public/custom-allocator --ignore public/call-runtime --ignore public/contract-xcm \ |
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 tests look like they pass (on my machine anyway) so we can enable them now?
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.
This is for building the risc-v example, I did you try that as well?
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.
Ah no sorry I did not see it was part of the riscv step
scope.append_encoded(&scale::Encode::encode(msg)); | ||
let enc_msg = scope.take_appended(); | ||
#[allow(deprecated)] | ||
ext::xcm_send(enc_dest, enc_msg, output.try_into().unwrap())?; |
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.
Could possibly use array_mut_ref!
for output
here?
Expose xcm_execute and xcm_send host functions to ink!