-
Notifications
You must be signed in to change notification settings - Fork 227
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
test: initial subxt test #990
Conversation
65a4cc4
to
a8d68ea
Compare
Started to use contract-transcode for encoding message selector, which works with the two exception:
|
I think this is a great effort overall!
Options I see so far:
U256 is not (yet) supported in |
4c58a90
to
9e9a8d8
Compare
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.
First review comments, looks good so far
9e9a8d8
to
ae22b0a
Compare
c993688
to
627ac9b
Compare
@extraymond it makes me sad that we didn't go forward sooner with this PR, sorry! I really like what you implemented here. In the long run, it would make sense to integrate this with #1061. How-ever, this should not block your nice work here. May I ask you for the favor of merging current |
Sure! I'll be on it sometime this week. |
Awesome, thanks! While you're on it, can you directly aim for compatibility with pallets contract v0.22.1 (it uses WeightV2)? Not all contracts will work ATM though, as I have to fix some imports first (gonna make a PR for this). EDIT: #1080 |
ea9c7d4
to
2b6b2af
Compare
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
@xermicus @seanyoung |
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
@extraymond FYI the rust source files need a |
Ah! Got it.
Ah got it, I'll fix it later. |
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.
Gave it a first round of review; looks great overall, I hope to merge this soon. Thank you so much for keeping this going! ❤️
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
@xermicus I think it would be nice to tap into debug buffer when contract trapped. It's very hard to debug why it's trapped within the test. |
I see what you mean; however I think this should ideally be done by For this to work we would need to provide the runtime debug info, but for now IIRC the contracts pallet doesn't support this. |
Yeah, cargo-contract does it by doing a sequential read call if a first write call failed. Maybe we should revisit it later when we have some shared lib from cargo-contract that we can directly pulled. |
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.
LGTM
Signed-off-by: Raymond Yeh <extraymond@gmail.com>
@seanyoung I think one of my forced push for rebase cleared your original change request. I believe I have implemented the suggestion back then. |
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.
LGTM
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 great. I only skimmed the new tests - a huge amount, great stuff!
BREAKING CHANGE: Signed-off-by: Raymond Yeh <extraymond@gmail.com>
Amazing, thank you! |
@extraymond good stuff, thanks! |
alternative tests using subxt, currently using raw selector until the ABI are compatible where we can just use contract-transcode.
test cases are semantically derived from the original substrate integration tests using polkadot.js.