Fix bugs in two examples + Fix master CI
#820
Conversation
It uses `debug_println!`.
It needs to be possible to send value with the transaction, because `t.transferred_value` in this function could be set to > 0.
Clippy fails otherwise: = note: `-D clippy::enum-variant-names` implied by `-D warnings` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
This reverts commit 9b051d2.
It needs to be possible to send value with the transaction, because `t.transferred_value` in this function could be set to > 0.
| /// Its return value indicates whether the called transaction was successful. | ||
| /// This can be called by anyone. | ||
| #[ink(message)] | ||
| #[ink(message, payable)] | ||
| pub fn invoke_transaction( | ||
| &mut self, | ||
| trans_id: TransactionId, | ||
| ) -> Result<(), Error> { |
There was a problem hiding this comment.
As a side note: There is another issue with this method. It internally uses
.map_err(|_| Error::TransactionFailed);
which currently would not revert the transaction. This will only happen once we have implemented error forwarding.
Currently, in this error case, the transaction would be taken (i.e. removed) out of the StorageStash, but not be executed due to the error. It would then not be possible to give the execution another try, since the transaction has been removed.
There was a problem hiding this comment.
Yeah this is a very important point made and acts as yet another motivator to implement storage reversal on error for non-ink-as-dependency calls. In the meantime we are required to panic in these critical cases.
cmichi
force-pushed
the
cmichi-misc-fixes
branch
7 times, most recently
from
fd13c96 to
3891d79
Compare
| ink_env = { version = "3.0.0-rc3", path = "../../crates/env", default-features = false } | ||
| ink_env = { version = "3.0.0-rc3", path = "../../crates/env", default-features = false, features = [ "ink-debug" ] } |
There was a problem hiding this comment.
A better way was to introduce an ink-debug feature and forward that to ink_env.
There was a problem hiding this comment.
So the problem with this approach is that cargo-contract builds with --no-default-features. But there is a ticket to forward an ink-debug feature: use-ink/cargo-contract#284.
But for now it means that if somebody builds contract-transfer with cargo-contract right now we need to explicitly enable the feature for ink_env like done in this PR, otherwise the user will not see the debug output.
There was a problem hiding this comment.
We should raise the priority of the --debug flag on cargo-contract then.
| /// Its return value indicates whether the called transaction was successful. | ||
| /// This can be called by anyone. | ||
| #[ink(message)] | ||
| #[ink(message, payable)] | ||
| pub fn invoke_transaction( | ||
| &mut self, | ||
| trans_id: TransactionId, | ||
| ) -> Result<(), Error> { |
There was a problem hiding this comment.
Yeah this is a very important point made and acts as yet another motivator to implement storage reversal on error for non-ink-as-dependency calls. In the meantime we are required to panic in these critical cases.
This reverts commit e61673a.
masterCI (clippy).ink-waterfalltests for those examples.In the case of
contract-transferI added a waterfall test which also checks if thedebug_println!in that contract example actually appears on the console.