Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(AU): charge storage fee and add initial CE #1058
feat(AU): charge storage fee and add initial CE #1058
Changes from 14 commits
7364f63
47b75ef
283f116
cb89df4
dde42c1
18cb574
a3787cd
8e6c148
c5b1f07
6744d84
09b30d8
66c6235
7a46c49
880d595
2693743
942c318
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 doesn't look really Rust-like to me.
Returning an
enum
would be more appropriate: (Mapped(address), Default(address)
).That also makes it more extensible for the future (not sure how we'd need it, but in general).
Anyhow, if others are ok with the proposed solution, you can ignore this comment.
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 like the rust way too.
I'll update the PR tomorrow with changes.
cc: @PierreOssun @shaunxw
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.
Agree, the
enum
one looks better.@PierreOssun the CE interface in ink! can get the decoded
enum
type without extra work in SDK right? Pls correct me if I'm wrong.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've updated the CE with enum, please have a look
Yes, as long as it supports scale decode it'll be fine, I've updated the test ink! contracts here.