block ordering and tx inclusion action rules #1412
Comments
|
regarding #2 (bridge control):
I don't think these necessarily need to go at the end of the block as they don't affect fees/fee recipient. having implications on validity seems fine, as other actions can also do that (eg transfers) |
transfers are much easier to account for in mempool design by having a pending balance and will generally not lead to otherwise valid txs being kicked out of the mempool. Want to avoid what could be a demotion from being a kicked out of mempool tx. If we already have multiple priority categories it is an improvement to have these after in terms of UX, I don't see a reason to not have as it's own considered type/priority. I think this alone wouldn't necessarily push us to implement these but if doing so we should do it. |
## Note for Reviewers Most of the logic changes are in the files `crates/astria-core/src/protocol/transaction/v1alpha1/action_group.rs` and `crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs`. The rest of the changes are updating the construction of `UnsignedTransactions` and/or changing the access to the inner fields. ## Summary Adds restrictions on what type of `Actions` can be included in the same `UnsignedTransaction`. Implements the categories as described in #1412 but with slightly different names: - General: - TransferAction - SequenceAction - BridgeLockAction - BridgeUnlockAction - IbcRelay - Ics20Withdrawal - ValidatorUpdate - UnbundeableGeneral (Instead of Bridge Control): - InitBridgeAccountAction - BridgeSudoChangeAction - Sudo: - FeeAssetChangeAction - FeeChangeAction - IbcRelayerChangeAction - UnbundleableSudo (Instead of Sudo Control): - SudoAddressChangeAction - IbcSudoChangeAction The check is applied at the time of constructing the `UnsignedTransaction`. The `UnsignedTransaction` additionally had its struct fields changed to private and now uses a new constructor to prevent the contained actions from being modified. ## Background We want transactions that can affect the validity of other transactions to be ordered last in blocks to reduce the amount of failed transactions we process. These logic changes are the first code changes being made to realize this goal. ## Changes - Introduced the `Actions` struct to hold valid groupings of `Actions`. - Introduced `ActionGroup` enum to represent which `Actions` can be included together in a transaction and if more than one action can be included in a transaction. - Changed the `UnsignedTransaction` struct to have private fields and to use a new constructor. - Changed the `UnsignedTransaction`'s `action` to be a `Actions` type instead of just a vector of `Actions`. ## Testing Unit testing and ran locally. ## Breaking Changelist Transactions that contain invalid `ActionGroup` combos (due to mixed groups or multiple actions in non-bundleable group types) are now 'invalid' transactions. I had to update one of the snapshot tests due to having to unbundle some of the transactions, creating a new state. ## Related Issues Initial steps for #1412 closes #1414, #1416
Sudo actions have some interesting side effects to the rest of the block, and as such should be posted at the bottom of the block with no non sudo actions occuring after them. We may even want a hierarchical order in which groupings can occur, and rules about coupling:
I propose the following transaction groupings, all actions in the same groupings have a property of if they can be bundled. If they can, they can only be bundled with same grouping txs. They are in order of how they should appear in a block.
TransferActionSequenceActionBridgeLockActionBridgeUnlockActionIbcRelayIcs20WithdrawalValidatorUpdateInitBridgeAccountActionBridgeSudoChangeActionIbcRelayactions as such should be towards end of block.FeeAssetChangeActionFeeChangeActionIbcRelayerChangeActionSudoAddressChangeActionI think these might be best viewed as a sort of "action category" where a transaction can only have actions that are all the same category, and categories have two properties (priority, and if it can be bundled). Thus the category of the first action would the that for the whole transaction, and stateless checks can easily be added over this.
┆Issue Number: ENG-750
The text was updated successfully, but these errors were encountered: