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

Unique trait id as a parameter for CallBuilder trait impls #1141

Merged
merged 11 commits into from
Feb 21, 2022

Conversation

ascjones
Copy link
Collaborator

Yet another solution for #982, as an alternative to #983 and #1136.

It is similar to #1136 except that it uses a unique id generated for the trait an by XOR of the trait selectors, which should be unique. As suggested by @xgreenx.

@ascjones ascjones changed the title Use unique trait id as a const generic parameter for CallBuilder trait impls Use unique trait id as a generic parameter for CallBuilder trait impls Feb 18, 2022
@ascjones ascjones changed the title Use unique trait id as a generic parameter for CallBuilder trait impls Use unique trait id as a parameter for CallBuilder trait impls Feb 18, 2022
@ascjones ascjones changed the title Use unique trait id as a parameter for CallBuilder trait impls Unique trait id as a parameter for CallBuilder trait impls Feb 18, 2022
@paritytech-cicd-pr
Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the examples/* contracts from this branch with cargo-contract 0.17.0-438b31d and comparing them to ink! master:

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator 1.35 K
adder 2.47 K
contract-terminate 1.23 K 215_704
contract-transfer 8.47 K 15_704
delegator 6.86 K 50_833
dns 9.70 K 47_112
erc1155 28.12 K 94_224
erc20 9.34 K 47_112
erc721 13.17 K 125_632
flipper 1.63 K 15_704
incrementer 1.56 K 15_704
multisig 26.25 K 102_922
proxy 3.36 K 32_147
rand-extension 4.64 K 15_704
subber 2.48 K
trait-erc20 9.64 K 47_112
trait-flipper 1.41 K 15_704
trait-incrementer 1.54 K 31_408

Link to the run | Last update: Fri Feb 18 18:43:11 CET 2022

Copy link
Collaborator

@xgreenx xgreenx left a comment

Choose a reason for hiding this comment

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

I'm okay with all three changes=) But between second and third I prefer third=)

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Yeah, this is my preferred solution as well!

examples/trait-incrementer/traits/Cargo.toml Outdated Show resolved Hide resolved
examples/trait-incrementer/traits/Cargo.toml Outdated Show resolved Hide resolved
examples/trait-incrementer/traits/lib.rs Outdated Show resolved Hide resolved
examples/trait-incrementer/Cargo.toml Outdated Show resolved Hide resolved
examples/trait-incrementer/Cargo.toml Outdated Show resolved Hide resolved
ascjones and others added 5 commits February 21, 2022 09:04
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
@ascjones ascjones merged commit 2f86746 into master Feb 21, 2022
@ascjones ascjones deleted the aj/call-builder-conflicting-trait-impls-trait-id branch February 21, 2022 09:49
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.

None yet

5 participants