-
Notifications
You must be signed in to change notification settings - Fork 432
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
Fix build warnings + clippy errors from latest nightly #686
Changes from all commits
00ea903
ebd2614
fdad2e8
c63ca4b
2a6565e
e67b499
6f050cd
423c8ae
6197d2a
fea57c0
a06c3d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ use crate::{ | |
Result, | ||
}; | ||
use ink_prelude::string::String; | ||
use std::str::FromStr; | ||
|
||
/// Pushes a contract execution context. | ||
/// | ||
|
@@ -420,11 +421,18 @@ pub fn assert_contract_termination<T, F>( | |
{ | ||
let value_any = ::std::panic::catch_unwind(should_terminate) | ||
.expect_err("contract did not terminate"); | ||
let encoded_input: &Vec<u8> = value_any | ||
.downcast_ref::<Vec<u8>>() | ||
let encoded_input = value_any | ||
.downcast_ref::<String>() | ||
.expect("panic object can not be cast"); | ||
let deserialized_vec = encoded_input | ||
.replace("[", "") | ||
.replace("]", "") | ||
.split(", ") | ||
.map(|s| u8::from_str(s).expect("u8 cannot be extracted from str")) | ||
.collect::<Vec<u8>>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dislike this hacky way of deserializing, but AFAIK it's the simplest way of getting the inverse of a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you create an issue for that so that we do not forget? |
||
let res: ContractTerminationResult<T> = | ||
scale::Decode::decode(&mut &encoded_input[..]).expect("input can not be decoded"); | ||
scale::Decode::decode(&mut &deserialized_vec[..]) | ||
.expect("input can not be decoded"); | ||
|
||
assert_eq!(res.beneficiary, expected_beneficiary); | ||
assert_eq!(res.transferred, expected_balance); | ||
|
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.
Maybe this actually should be converted into an
unimplemented!
with a proper message stating why it is still unimeplemented?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.
Hmm, so I'm not sure.
I mean, even if we implement setting a tombstone, clearing storage, etc. we would still like to return the result of the contract eviction and it seems that
panic!
is the right way to go.From my pov your suggestion would just be a semantic change of having something like
unimplemented!("we have not yet implemented everything, but anyway here's the result: …")
instead and changing that back topanic!(…)
once we have implemented it would then be a breaking change.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.
Well hopefully this discussion is not needed soon due to the works on the upcoming off-chain environment. :P But since contract termination is not really properly implemented in the current off-chain environment I would not dare to actually just mark it as unimplemented. But yeah let's keep it as it is for now.