Add Enable tips parameter #142
Conversation
faddat
requested review from
aljo242,
Eric-Warehime,
technicallyty and
wesl-ee
as code owners
|
@faddat thanks for opening this! Could we add some tests to |
|
Absolutely, id be very happy to! |
|
So it seems the conditional adds 18 gas -- next commit fixes existing tests.... (by adding 18 gas to them) |
…action_-should_pass_and_skip_deduction_until_next_block
|
@aljo242 - I think this is good to go. Only thing I am unsure about right now is if I should make 18 a constant, continue to use it as a bare number, or something else |
| @@ -353,7 +406,7 @@ func TestPostHandleMock(t *testing.T) { | |||
| Simulate: false, | |||
| ExpPass: true, | |||
| ExpErr: nil, | |||
| ExpectConsumedGas: 15340, // extra gas consumed because msg server is run, but deduction is skipped | |||
| ExpectConsumedGas: 15412, // extra gas consumed because msg server is run, but deduction is skipped | |||
| proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) | ||
| if !tip.IsNil() { | ||
| // Only process tips if EnableTips is true | ||
| if params.EnableTips && !tip.IsNil() { |
There was a problem hiding this comment.
If tips is not enabled, then, fee+tip should be passed to DeductCoins above
There was a problem hiding this comment.
The full value is already held in the escrow account, so it needs to be accounted for and not refunded
There was a problem hiding this comment.
I think this is set now, and thank you!
|
@faddat we are not going to be adding any features to |
|
@aljo242 thanks so much for the recommended changes, and for you and Skips work on fee market. It's great! I'll correct this nonetheless. :) |
|
thank you so much for this. Do you have any suggestion on how to test for this? Because another thing that we learned here is that the tests that I wrote are probably not good enough. |
This PR provides a new parameter, enable_tips.
This way, it is not always necessary to support the tip concept.