Skip to content

Commit

Permalink
chore(sequencer): index all ABCI events (#1786)
Browse files Browse the repository at this point in the history
## Summary
Instructs CometBFT to index all ABCI events emitted by Sequencer.

## Background
This change instructs CometBFT to index all events emitted by Sequencer
by setting their field `index: true`.

This allows programatically determining stuck IBC transactions to
automatically trigger a timeout. Previously, the height that a given IBC
packet was sent at had to be found manually before it could be passed to
the Hermes CLI (Astria's IBC relayer of choice).

The indexing field is described in the CometBFT ABCI spec [1].

1:
https://github.com/cometbft/cometbft/blob/16b1ed87c4b10118df10eb7175412db066bd09d5/spec/abci/abci%2B%2B_basic_concepts.md?plain=1#L318-L319

## Changes
- Index all event attributes before returning in
`app::execute_transaction`

## Testing
- Added unit test to ensure all event attributes are indexed for the
events we create (unable to unit test an IBC action).

## Changelogs
Changelog updated
  • Loading branch information
ethanoroshiba authored Nov 15, 2024
1 parent 01dcf49 commit 4564fc3
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 19 deletions.
4 changes: 4 additions & 0 deletions crates/astria-sequencer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## Changed

- Index all event attributes [#1786](https://github.com/astriaorg/astria/pull/1786).

## [1.0.0] - 2024-10-25

### Changed
Expand Down
11 changes: 10 additions & 1 deletion crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,16 @@ impl App {
.iter()
.any(|act| act.is_fee_asset_change() || act.is_fee_change());

Ok(state_tx.apply().1)
// index all event attributes
let mut events = state_tx.apply().1;
for event in &mut events {
event
.attributes
.iter_mut()
.for_each(|attr| attr.index = true);
}

Ok(events)
}

#[instrument(name = "App::end_block", skip_all)]
Expand Down
51 changes: 51 additions & 0 deletions crates/astria-sequencer/src/app/tests_execute_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1278,3 +1278,54 @@ async fn transaction_execution_records_fee_event() {
assert_eq!(event.attributes[2].key, "feeAmount");
assert_eq!(event.attributes[3].key, "positionInTransaction");
}

#[tokio::test]
async fn ensure_all_event_attributes_are_indexed() {
let mut app = initialize_app(None, vec![]).await;
let mut state_tx = StateDelta::new(app.state.clone());

let alice = get_alice_signing_key();
let bob_address = astria_address_from_hex_string(BOB_ADDRESS);
let value = 333_333;
state_tx
.put_bridge_account_rollup_id(&bob_address, [0; 32].into())
.unwrap();
state_tx.put_allowed_fee_asset(&nria()).unwrap();
state_tx
.put_bridge_account_ibc_asset(&bob_address, nria())
.unwrap();
app.apply(state_tx);

let transfer_action = Transfer {
to: bob_address,
amount: value,
asset: nria().into(),
fee_asset: nria().into(),
};
let bridge_lock_action = BridgeLock {
to: bob_address,
amount: 1,
asset: nria().into(),
fee_asset: nria().into(),
destination_chain_address: "test_chain_address".to_string(),
};
let tx = TransactionBody::builder()
.actions(vec![transfer_action.into(), bridge_lock_action.into()])
.chain_id("test")
.try_build()
.unwrap();

let signed_tx = Arc::new(tx.sign(&alice));
let events = app.execute_transaction(signed_tx).await.unwrap();

events
.iter()
.flat_map(|event| &event.attributes)
.for_each(|attribute| {
assert!(
attribute.index,
"attribute {} is not indexed",
attribute.key,
);
});
}
13 changes: 5 additions & 8 deletions crates/astria-sequencer/src/fees/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ use cnidarium::{
};
use futures::Stream;
use pin_project_lite::pin_project;
use tendermint::abci::{
Event,
EventAttributeIndexExt as _,
};
use tendermint::abci::Event;
use tracing::instrument;

use super::{
Expand Down Expand Up @@ -574,10 +571,10 @@ fn construct_tx_fee_event(fee: &Fee) -> Event {
Event::new(
"tx.fees",
[
("actionName", fee.action_name.to_string()).index(),
("asset", fee.asset.to_string()).index(),
("feeAmount", fee.amount.to_string()).index(),
("positionInTransaction", fee.source_action_index.to_string()).index(),
("actionName", fee.action_name.to_string()),
("asset", fee.asset.to_string()),
("feeAmount", fee.amount.to_string()),
("positionInTransaction", fee.source_action_index.to_string()),
],
)
}
Expand Down
17 changes: 7 additions & 10 deletions crates/astria-sequencer/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use astria_eyre::eyre::{
};
use tendermint::abci::{
self,
EventAttributeIndexExt as _,
};

pub(crate) fn cometbft_to_sequencer_validator(
Expand All @@ -31,21 +30,19 @@ pub(crate) fn create_deposit_event(deposit: &Deposit) -> abci::Event {
abci::Event::new(
"tx.deposit",
[
("bridgeAddress", deposit.bridge_address.to_string()).index(),
("rollupId", deposit.rollup_id.to_string()).index(),
("amount", deposit.amount.to_string()).index(),
("asset", deposit.asset.to_string()).index(),
("bridgeAddress", deposit.bridge_address.to_string()),
("rollupId", deposit.rollup_id.to_string()),
("amount", deposit.amount.to_string()),
("asset", deposit.asset.to_string()),
(
"destinationChainAddress",
deposit.destination_chain_address.to_string(),
)
.index(),
),
(
"sourceTransactionId",
deposit.source_transaction_id.to_string(),
)
.index(),
("sourceActionIndex", deposit.source_action_index.to_string()).index(),
),
("sourceActionIndex", deposit.source_action_index.to_string()),
],
)
}
Expand Down

0 comments on commit 4564fc3

Please sign in to comment.