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 chain extension example to show argument passing #1029

Merged
merged 5 commits into from
Nov 24, 2021

Conversation

justinfrevert
Copy link
Contributor

This changes the rand-extension Ink! example code to pass along an argument from the contract to the chain extension in the runtime. This PR addresses the first ToDo here, which is a repeat user question. I verified this works in a Substrate node on my machine.

@paritytech-ci
Copy link

paritytech-ci commented Nov 22, 2021

🦑 📈 ink! Example Contracts ‒ Size Change Report 📉 🦑

These are the results of building the examples/* contracts from this branch with cargo-contract 0.15.0:

Δ Original Size Δ Optimized Size Total Optimized Size
accumulator 6.20 K
adder 7.43 K
contract-terminate 1.50 K
contract-transfer 7.73 K
delegator 10.02 K
dns 21.07 K
erc1155 44.90 K
erc20 10.17 K
erc721 35.20 K
flipper 2.02 K
incrementer 1.92 K
multisig 44.85 K
rand-extension +0.45 K +0.41 K 5.08 K
subber 7.44 K
trait-erc20 26.48 K
trait-flipper 1.83 K
trait-incrementer 1.96 K

Link to the run | Last update: Wed Nov 24 07:11:14 CET 2021

@justinfrevert
Copy link
Contributor Author

The failing CI step seems to be unrelated to this change and prevalent among other current PRs.

@@ -88,9 +88,9 @@ mod rand_extension {

/// Update the value from the runtimes random source.
Copy link
Collaborator

@cmichi cmichi Nov 23, 2021

Choose a reason for hiding this comment

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

Could you update this comment here and describe the effect of the subject argument?

@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2021

Codecov Report

Merging #1029 (d8ae73f) into master (2ee203d) will decrease coverage by 3.74%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1029      +/-   ##
==========================================
- Coverage   78.87%   75.13%   -3.75%     
==========================================
  Files         246      246              
  Lines        9260     9258       -2     
==========================================
- Hits         7304     6956     -348     
- Misses       1956     2302     +346     
Impacted Files Coverage Δ
crates/lang/ir/src/ir/contract.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/ir/src/ir/ink_test.rs 0.00% <0.00%> (-87.50%) ⬇️
crates/eth_compatibility/src/lib.rs 0.00% <0.00%> (-74.08%) ⬇️
crates/lang/ir/src/ir/trait_def/mod.rs 0.00% <0.00%> (-66.67%) ⬇️
crates/lang/ir/src/ir/blake2.rs 16.66% <0.00%> (-62.50%) ⬇️
crates/lang/ir/src/ir/trait_def/item/trait_item.rs 46.03% <0.00%> (-44.45%) ⬇️
crates/lang/ir/src/ir/trait_def/config.rs 7.69% <0.00%> (-38.47%) ⬇️
crates/lang/ir/src/ir/item_mod.rs 59.73% <0.00%> (-32.22%) ⬇️
crates/lang/ir/src/ir/utils.rs 53.84% <0.00%> (-30.77%) ⬇️
crates/primitives/src/key_ptr.rs 75.00% <0.00%> (-25.00%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ee203d...d8ae73f. Read the comment docs.

@cmichi
Copy link
Collaborator

cmichi commented Nov 23, 2021

The failing CI step seems to be unrelated to this change and prevalent among other current PRs.

It's fixed now, it was an issue with one of the GitHub tokens.

@cmichi
Copy link
Collaborator

cmichi commented Nov 23, 2021

@justinfrevert Could you also merge master into your PR? We think the contract sizes which were posted might be due to your PR being behind master.

@justinfrevert
Copy link
Contributor Author

Thanks for grabbing it, @HCastano

Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

Thanks @justinfrevert!

@cmichi cmichi merged commit 3f0808b into master Nov 24, 2021
@cmichi cmichi deleted the frevert-chain-extension-example-arg branch November 24, 2021 06:10
@cmichi
Copy link
Collaborator

cmichi commented Nov 24, 2021

@justinfrevert Are you up for answering the StackExchange question as well?

@justinfrevert
Copy link
Contributor Author

@justinfrevert Are you up for answering the StackExchange question as well?

I gave it a shot. Let me know how that looks.

@cmichi
Copy link
Collaborator

cmichi commented Nov 24, 2021

Great, thx! We should also incorporate your answer into our ink-docs.

xgreenx pushed a commit to Supercolony-net/ink that referenced this pull request Feb 8, 2022
* Add argument to rand-extension example

* add appropriate runtime and test example changes

* Clarify rand-extension  comment wrt subject argument

* Update examples/rand-extension/lib.rs

Co-authored-by: Hernando Castano <[email protected]>
Co-authored-by: Michael Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants