Skip to content

Commit

Permalink
respond to comments by @SuperFluffy
Browse files Browse the repository at this point in the history
  • Loading branch information
Lilyjjo committed Sep 30, 2024
1 parent 1809748 commit 052dbfd
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 26 deletions.
7 changes: 6 additions & 1 deletion crates/astria-composer/src/executor/bundle_factory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ impl SizedBundle {
/// Constructs an [`UnsignedTransaction`] from the actions contained in the bundle and `params`.
// Method is expected to never panic because only `SequenceActions` are added to the bundle,
// which should produce a valid variant of the `ActionGroup` type.
#[allow(clippy::panic)]
#[allow(
clippy::panic,
reason = "method is expected to never panic because only `SequenceActions` are added to \
the bundle, which should produce a valid variant of the `ActionGroup` type; \
this is checked by `tests::transaction_construction_should_not_panic"
)]
pub(super) fn to_unsigned_transaction(
&self,
nonce: u32,
Expand Down
1 change: 0 additions & 1 deletion crates/astria-composer/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,6 @@ impl Future for SubmitFut {
type Output = eyre::Result<u32>;

// FIXME (https://github.com/astriaorg/astria/issues/1572): This function is too long and should be refactored.
#[expect(clippy::too_many_lines, reason = "this may warrant a refactor")]
fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll<Self::Output> {
const INVALID_NONCE: Code = Code::Err(AbciErrorCode::INVALID_NONCE.value());
loop {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ macro_rules! impl_belong_to_group {
}

impl_belong_to_group!(
(SequenceAction, ActionGroup::General),
(TransferAction, ActionGroup::General),
(ValidatorUpdate, ActionGroup::General),
(SequenceAction, ActionGroup::BundleableGeneral),
(TransferAction, ActionGroup::BundleableGeneral),
(ValidatorUpdate, ActionGroup::BundleableGeneral),
(SudoAddressChangeAction, ActionGroup::UnbundleableSudo),
(IbcRelayerChangeAction, ActionGroup::Sudo),
(Ics20Withdrawal, ActionGroup::General),
(IbcRelayerChangeAction, ActionGroup::BundleableSudo),
(Ics20Withdrawal, ActionGroup::BundleableGeneral),
(InitBridgeAccountAction, ActionGroup::UnbundleableGeneral),
(BridgeLockAction, ActionGroup::General),
(BridgeUnlockAction, ActionGroup::General),
(BridgeLockAction, ActionGroup::BundleableGeneral),
(BridgeUnlockAction, ActionGroup::BundleableGeneral),
(BridgeSudoChangeAction, ActionGroup::UnbundleableGeneral),
(FeeChangeAction, ActionGroup::Sudo),
(FeeAssetChangeAction, ActionGroup::Sudo),
(IbcRelay, ActionGroup::General),
(FeeChangeAction, ActionGroup::BundleableSudo),
(FeeAssetChangeAction, ActionGroup::BundleableSudo),
(IbcRelay, ActionGroup::BundleableGeneral),
(IbcSudoChangeAction, ActionGroup::UnbundleableSudo),
);

Expand All @@ -82,28 +82,31 @@ impl Action {

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(super) enum ActionGroup {
General,
BundleableGeneral,
UnbundleableGeneral,
Sudo,
BundleableSudo,
UnbundleableSudo,
}

impl ActionGroup {
pub(super) fn is_bundleable(self) -> bool {
matches!(self, ActionGroup::General | ActionGroup::Sudo)
matches!(
self,
ActionGroup::BundleableGeneral | ActionGroup::BundleableSudo
)
}

pub(super) fn is_sudo(self) -> bool {
matches!(self, ActionGroup::Sudo)
pub(super) fn is_bundleable_sudo(self) -> bool {
matches!(self, ActionGroup::BundleableSudo)
}
}

impl fmt::Display for ActionGroup {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ActionGroup::General => write!(f, "general"),
ActionGroup::BundleableGeneral => write!(f, "bundleable general"),
ActionGroup::UnbundleableGeneral => write!(f, "unbundleable general"),
ActionGroup::Sudo => write!(f, "sudo"),
ActionGroup::BundleableSudo => write!(f, "bundleable sudo"),
ActionGroup::UnbundleableSudo => write!(f, "unbundleable sudo"),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
const ASTRIA_ADDRESS_PREFIX: &str = "astria";

#[test]
fn try_from_list_of_actions_general() {
fn try_from_list_of_actions_bundleable_general() {
let address: Address<_> = Address::builder()
.array([0; 20])
.prefix(ASTRIA_ADDRESS_PREFIX)
Expand Down Expand Up @@ -92,12 +92,12 @@ fn try_from_list_of_actions_general() {

assert!(matches!(
Actions::try_from_list_of_actions(actions).unwrap().group(),
Some(ActionGroup::General)
Some(ActionGroup::BundleableGeneral)
));
}

#[test]
fn from_list_of_actions_sudo() {
fn from_list_of_actions_bundleable_sudo() {
let address: Address<_> = Address::builder()
.array([0; 20])
.prefix(ASTRIA_ADDRESS_PREFIX)
Expand All @@ -116,7 +116,7 @@ fn from_list_of_actions_sudo() {

assert!(matches!(
Actions::try_from_list_of_actions(actions).unwrap().group(),
Some(ActionGroup::Sudo)
Some(ActionGroup::BundleableSudo)
));
}

Expand Down
4 changes: 2 additions & 2 deletions crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ impl SignedTransaction {
}

#[must_use]
pub fn is_sudo_action_group(&self) -> bool {
pub fn is_bundleable_sudo_action_group(&self) -> bool {
if let Some(group) = self.transaction.actions.group() {
group.is_sudo()
group.is_bundleable_sudo()
} else {
false
}
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ impl App {

// flag mempool for cleaning if we ran a fee change action
self.recost_mempool = self.recost_mempool
|| signed_tx.is_sudo_action_group()
|| signed_tx.is_bundleable_sudo_action_group()
&& signed_tx
.actions()
.iter()
Expand Down

0 comments on commit 052dbfd

Please sign in to comment.