Skip to content

Fix ink! message result return reverts#998

Merged
cmichi merged 12 commits intomasterfrom
rf-fix-result-returns
Nov 8, 2021
Merged

Fix ink! message result return reverts#998
cmichi merged 12 commits intomasterfrom
rf-fix-result-returns

Conversation

@Robbepop
Copy link
Collaborator

@Robbepop Robbepop commented Nov 1, 2021

Fixes #985.

@xgreenx Please check if this fixes the problems for you.

Not yet optimized. Will do once we know for sure that this fix actually works.

Copy link
Contributor

@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.

It works=)

);
let result: #message_output = #message_callable(&mut contract, input);
let failure = ::ink_lang::is_result_type!(#message_output)
&& ::ink_lang::is_result_err!(&result);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&& ::ink_lang::is_result_err!(&result);
&& ::ink_lang::is_result_err!(result);

Copy link
Collaborator Author

@Robbepop Robbepop Nov 1, 2021

Choose a reason for hiding this comment

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

Thank you for the debugging help, @xgreenx !

@xgreenx
Copy link
Contributor

xgreenx commented Nov 1, 2021

I tested with Europa and you can see that return value flags: 1. So it is reverted
image

@Robbepop
Copy link
Collaborator Author

Robbepop commented Nov 1, 2021

On the downside of this PR is that it currently slightly increases the Wasm file sizes of some ink! smart contracts. E.g. ERC-20 raised from 28.9kB to 31.4kB in size. We might be able to find some optimizations to reduce this additional bloat.

@cmichi cmichi merged commit e26c350 into master Nov 8, 2021
@cmichi cmichi deleted the rf-fix-result-returns branch November 8, 2021 13:23
@cmichi cmichi restored the rf-fix-result-returns branch November 8, 2021 13:24
@cmichi cmichi deleted the rf-fix-result-returns branch November 9, 2021 10:43
@HCastano
Copy link
Contributor

HCastano commented Nov 9, 2021

On the downside of this PR is that it currently slightly increases the Wasm file sizes of some ink! smart contracts. E.g. ERC-20 raised from 28.9kB to 31.4kB in size. We might be able to find some optimizations to reduce this additional bloat.

I was scratching my head trying to figure out why the ERC-20 on master has increased in size, haha

xgreenx pushed a commit to Supercolony-net/ink that referenced this pull request Feb 8, 2022
* add initiate_message and finalize_message utility codegen methods

* make is_result_type and is_result_err usable from outside ink_lang

Hide their docs since they are not meant to be used by ink! users.

* use the new initiate_message and finalize_message in ink! codegen

* remove deprecated execute_message utility method

* apply rustfmt

* fix clippy warnings

* prevent Drop call for static storage

* add missing docs

* fix ui test

* fix macro hygiene problem

* fix bug

* replace some #[inline(always)] with #[inline]
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.

Feature revert_on_error doesn't work

4 participants