enum Pays for PaysFee #5733
enum Pays for PaysFee #5733
Conversation
kianenigma
added
A0-please_review
| @@ -1630,7 +1630,7 @@ fn cannot_self_destruct_in_constructor() { | |||
| #[test] | |||
| fn check_block_gas_limit_works() { | |||
| ExtBuilder::default().block_gas_limit(50).build().execute_with(|| { | |||
| let info = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: true }; | |||
| let info = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes }; | |||
There was a problem hiding this comment.
So except for the high level bin/node/executor tests, I will keep this assertions explicit intentionally, instead of doing Default::default() which is unclear and flaky.
|
Is it actually more clear though? I don't yet see the advantage, unless we expect there to be another option besides yes and no :) |
|
honestly I can't think of a third option now. I was recommended to do this by @bkchr and to me it just made sense as an assurance and being more explicit. |
The point is, if you read the code you will directly understand what this parameter means, without opening the docs and find the place where this is documented. |
|
FYI, this is a backwards incompatible change in the way this is encoded in SCALE for non-Rust clients. Where previously |
|
@maciejhirsz Thanks for picking this up. Yeap, the API decoding broken with this, so will have to do an update. |
|
@maciejhirsz @jacogr does it make sense to just flip the order of the enum here? |
|
This is already deployed on Kusama, so no. |
OMG, I debug this for a whole afternoon. |
* enum Pays for PaysFee * Fix doc test * Update bin/node/executor/tests/basic.rs * Update bin/node/executor/tests/basic.rs
small follow up for #5686
enum Paysto be more clear with payment.