feat(sequencer)!: implement FeeChangeAction for the authority component
#1037
Conversation
github-actions
bot
added
proto
| // this to accomodate both `base_fee` and `byte_cost_multiplier` for each action. | ||
| oneof value { | ||
| // core protocol fees are defined on 1-20 | ||
| bytes transfer_base_fee = 1; |
There was a problem hiding this comment.
Should these by bytes or typed as u128 or uint64?
It might be cleaner to have them all be uint128 and then because it's a message they are optional. could get rid of the oneof, can update as many as you want at once anything not set stays the same?
There was a problem hiding this comment.
updated to make each oneof value type uint128 - i think it's better to leave it as a oneof for now, as changing multiple fees in one action can get more complex than just bundling a tx with multiple FeeChangeActions in it.
bharath-123
force-pushed
the
noot/fee-change-action
branch
from
295c3b8 to
788ebba
Compare
noot
force-pushed
the
noot/fee-change-action
branch
from
788ebba to
295c3b8
Compare
| // for all actions in the transaction. | ||
| // | ||
| // allow too many lines because of the number of actions and the need to check each one. | ||
| #[allow(clippy::too_many_lines)] |
There was a problem hiding this comment.
IMO this is a strong hint to have a bunch of free-standing functions for each of the actions, and delegate the checks there. I.e:
match action {
Action::Transfer(act) => process_transfer_action(act, fees_by_asset), // or some better name obviously
...
}This would also make it possible to unit-testing these individually.
| sequence::state_ext::StateWriteExt as _, | ||
| }; | ||
|
|
||
| match self.fee_change { |
There was a problem hiding this comment.
do ever get events for this? I wonder if it might be warranted to emit warn! level events for each one of these changes.
There was a problem hiding this comment.
yeah, Jordan and I discussed this, it might be useful to emit ABCI events for certain things occurring for easier indexing in general. this would be one of those spots. will probably look over all of sequencer at some point to decide what to add
Summary
implement
FeeChangeActionfor the authority component, which allows fees to be configured at chain runtime.Background
we want to allow for fees to be configured at chain runtime.
Changes
FeeChangeActiontypeTesting
unit tests
Breaking Changelist
FeeChangeAction.Related Issues
closes #1003