Skip to content

Commit ac7222e

Browse files
chore(sequencer): migrate from anyhow::Result to eyre::Result (#1387)
## Summary Migrate all instances of `anyhow::Result` to `eyre::Result`. ## Background Sequencer was using `anyhow::Result`, which provides an unhelpful `Display` impl and contrasts our error handling in the rest of the codebase. This change is to flush out our error handling in the sequencer, except for those parts which necessitate using `anyhow`. ## Changes - Add eyre to sequencer's `cargo.toml`. - Migrate all instances of `anyhow::Result` to `eyre::Result`, except for those that touch cnidarium directly. - Create `anyhow_to_eyre()` and `eyre_to_anyhow()` helper functions for moving between the two without breaking the source chain. ## Testing Added unit tests to ensure `eyre` and `anyhow` source chains are maintained when converting between one another. ## Related Issues closes #1386
1 parent 0a83cae commit ac7222e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+890
-672
lines changed

Cargo.lock

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/astria-eyre/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edition = "2021"
88
[dependencies]
99
eyre = "0.6"
1010
itoa = "1.0.10"
11+
anyhow = { version = "1.0.0", optional = true }
1112

1213
[dev-dependencies]
1314
tracing = { workspace = true }

crates/astria-eyre/src/lib.rs

+48
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ use std::{
55
fmt::Write as _,
66
};
77

8+
#[cfg(feature = "anyhow")]
9+
pub use anyhow;
10+
#[cfg(feature = "anyhow")]
11+
pub use anyhow_conversion::{
12+
anyhow_to_eyre,
13+
eyre_to_anyhow,
14+
};
815
pub use eyre;
916
#[doc(hidden)]
1017
pub use eyre::Result;
@@ -83,3 +90,44 @@ fn write_value(err: &dyn Error, f: &mut core::fmt::Formatter<'_>) -> core::fmt::
8390
f.write_fmt(format_args!("\"{err}\""))?;
8491
Ok(())
8592
}
93+
94+
#[cfg(feature = "anyhow")]
95+
mod anyhow_conversion {
96+
pub fn anyhow_to_eyre(anyhow_error: anyhow::Error) -> eyre::Report {
97+
let boxed: Box<dyn std::error::Error + Send + Sync> = anyhow_error.into();
98+
eyre::eyre!(boxed)
99+
}
100+
101+
#[must_use]
102+
pub fn eyre_to_anyhow(eyre_error: eyre::Report) -> anyhow::Error {
103+
let boxed: Box<dyn std::error::Error + Send + Sync> = eyre_error.into();
104+
anyhow::anyhow!(boxed)
105+
}
106+
107+
#[cfg(test)]
108+
mod test {
109+
#[test]
110+
fn anyhow_to_eyre_preserves_source_chain() {
111+
let mut errs = ["foo", "bar", "baz", "qux"];
112+
let anyhow_error = anyhow::anyhow!(errs[0]).context(errs[1]).context(errs[2]);
113+
let eyre_from_anyhow = super::anyhow_to_eyre(anyhow_error).wrap_err(errs[3]);
114+
115+
errs.reverse();
116+
for (i, err) in eyre_from_anyhow.chain().enumerate() {
117+
assert_eq!(errs[i], &err.to_string());
118+
}
119+
}
120+
121+
#[test]
122+
fn eyre_to_anyhow_preserves_source_chain() {
123+
let mut errs = ["foo", "bar", "baz", "qux"];
124+
let eyre_error = eyre::eyre!(errs[0]).wrap_err(errs[1]).wrap_err(errs[2]);
125+
let anyhow_from_eyre = super::eyre_to_anyhow(eyre_error).context(errs[3]);
126+
127+
errs.reverse();
128+
for (i, err) in anyhow_from_eyre.chain().enumerate() {
129+
assert_eq!(errs[i], &err.to_string());
130+
}
131+
}
132+
}
133+
}

crates/astria-sequencer/Cargo.toml

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@ benchmark = ["divan"]
1717
[dependencies]
1818
astria-core = { path = "../astria-core", features = ["server", "serde"] }
1919
astria-build-info = { path = "../astria-build-info", features = ["runtime"] }
20+
21+
# The "anyhow" feature is only included because it is necessary for the implementation of
22+
# `penumbra_ibc::component::HostInterface` in `crates/astria-sequencer/src/ibc/host_interface.rs`.
23+
# Avoid using "anyhow" results anywhere else.
24+
astria-eyre = { path = "../astria-eyre", features = ["anyhow"] }
25+
2026
config = { package = "astria-config", path = "../astria-config" }
2127
merkle = { package = "astria-merkle", path = "../astria-merkle" }
2228
telemetry = { package = "astria-telemetry", path = "../astria-telemetry", features = [
2329
"display",
2430
] }
25-
26-
anyhow = "1"
2731
borsh = { version = "1", features = ["derive"] }
2832
cnidarium = { git = "https://github.com/penumbra-zone/penumbra.git", rev = "87adc8d6b15f6081c1adf169daed4ca8873bd9f6", features = [
2933
"metrics",

crates/astria-sequencer/src/accounts/action.rs

+21-20
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
use anyhow::{
2-
ensure,
3-
Context,
4-
Result,
5-
};
61
use astria_core::{
72
protocol::transaction::v1alpha1::action::TransferAction,
83
Protobuf as _,
94
};
5+
use astria_eyre::eyre::{
6+
ensure,
7+
OptionExt as _,
8+
Result,
9+
WrapErr as _,
10+
};
1011
use cnidarium::{
1112
StateRead,
1213
StateWrite,
@@ -44,7 +45,7 @@ impl ActionHandler for TransferAction {
4445
state
4546
.get_bridge_account_rollup_id(from)
4647
.await
47-
.context("failed to get bridge account rollup id")?
48+
.wrap_err("failed to get bridge account rollup id")?
4849
.is_none(),
4950
"cannot transfer out of bridge account; BridgeUnlock must be used",
5051
);
@@ -60,7 +61,7 @@ pub(crate) async fn execute_transfer<S, TAddress>(
6061
action: &TransferAction,
6162
from: TAddress,
6263
mut state: S,
63-
) -> anyhow::Result<()>
64+
) -> Result<()>
6465
where
6566
S: StateWrite,
6667
TAddress: AddressBytes,
@@ -70,11 +71,11 @@ where
7071
let fee = state
7172
.get_transfer_base_fee()
7273
.await
73-
.context("failed to get transfer base fee")?;
74+
.wrap_err("failed to get transfer base fee")?;
7475
state
7576
.get_and_increase_block_fees(&action.fee_asset, fee, TransferAction::full_name())
7677
.await
77-
.context("failed to add to block fees")?;
78+
.wrap_err("failed to add to block fees")?;
7879

7980
// if fee payment asset is same asset as transfer asset, deduct fee
8081
// from same balance as asset transferred
@@ -88,28 +89,28 @@ where
8889
state
8990
.decrease_balance(from, &action.asset, payment_amount)
9091
.await
91-
.context("failed decreasing `from` account balance")?;
92+
.wrap_err("failed decreasing `from` account balance")?;
9293
state
9394
.increase_balance(action.to, &action.asset, action.amount)
9495
.await
95-
.context("failed increasing `to` account balance")?;
96+
.wrap_err("failed increasing `to` account balance")?;
9697
} else {
9798
// otherwise, just transfer the transfer asset and deduct fee from fee asset balance
9899
// later
99100
state
100101
.decrease_balance(from, &action.asset, action.amount)
101102
.await
102-
.context("failed decreasing `from` account balance")?;
103+
.wrap_err("failed decreasing `from` account balance")?;
103104
state
104105
.increase_balance(action.to, &action.asset, action.amount)
105106
.await
106-
.context("failed increasing `to` account balance")?;
107+
.wrap_err("failed increasing `to` account balance")?;
107108

108109
// deduct fee from fee asset balance
109110
state
110111
.decrease_balance(from, &action.fee_asset, fee)
111112
.await
112-
.context("failed decreasing `from` account balance for fee payment")?;
113+
.wrap_err("failed decreasing `from` account balance for fee payment")?;
113114
}
114115
Ok(())
115116
}
@@ -123,35 +124,35 @@ where
123124
S: StateRead,
124125
TAddress: AddressBytes,
125126
{
126-
state.ensure_base_prefix(&action.to).await.context(
127+
state.ensure_base_prefix(&action.to).await.wrap_err(
127128
"failed ensuring that the destination address matches the permitted base prefix",
128129
)?;
129130
ensure!(
130131
state
131132
.is_allowed_fee_asset(&action.fee_asset)
132133
.await
133-
.context("failed to check allowed fee assets in state")?,
134+
.wrap_err("failed to check allowed fee assets in state")?,
134135
"invalid fee asset",
135136
);
136137

137138
let fee = state
138139
.get_transfer_base_fee()
139140
.await
140-
.context("failed to get transfer base fee")?;
141+
.wrap_err("failed to get transfer base fee")?;
141142
let transfer_asset = action.asset.clone();
142143

143144
let from_fee_balance = state
144145
.get_account_balance(&from, &action.fee_asset)
145146
.await
146-
.context("failed getting `from` account balance for fee payment")?;
147+
.wrap_err("failed getting `from` account balance for fee payment")?;
147148

148149
// if fee asset is same as transfer asset, ensure accounts has enough funds
149150
// to cover both the fee and the amount transferred
150151
if action.fee_asset.to_ibc_prefixed() == transfer_asset.to_ibc_prefixed() {
151152
let payment_amount = action
152153
.amount
153154
.checked_add(fee)
154-
.context("transfer amount plus fee overflowed")?;
155+
.ok_or_eyre("transfer amount plus fee overflowed")?;
155156

156157
ensure!(
157158
from_fee_balance >= payment_amount,
@@ -168,7 +169,7 @@ where
168169
let from_transfer_balance = state
169170
.get_account_balance(from, transfer_asset)
170171
.await
171-
.context("failed to get account balance in transfer check")?;
172+
.wrap_err("failed to get account balance in transfer check")?;
172173
ensure!(
173174
from_transfer_balance >= action.amount,
174175
"insufficient funds for transfer"

crates/astria-sequencer/src/accounts/component.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use std::sync::Arc;
22

3-
use anyhow::{
4-
Context,
3+
use astria_core::protocol::genesis::v1alpha1::GenesisAppState;
4+
use astria_eyre::eyre::{
55
Result,
6+
WrapErr as _,
67
};
7-
use astria_core::protocol::genesis::v1alpha1::GenesisAppState;
88
use tendermint::abci::request::{
99
BeginBlock,
1010
EndBlock,
@@ -32,16 +32,16 @@ impl Component for AccountsComponent {
3232
let native_asset = state
3333
.get_native_asset()
3434
.await
35-
.context("failed to read native asset from state")?;
35+
.wrap_err("failed to read native asset from state")?;
3636
for account in app_state.accounts() {
3737
state
3838
.put_account_balance(account.address, &native_asset, account.balance)
39-
.context("failed writing account balance to state")?;
39+
.wrap_err("failed writing account balance to state")?;
4040
}
4141

4242
state
4343
.put_transfer_base_fee(app_state.fees().transfer_base_fee)
44-
.context("failed to put transfer base fee")?;
44+
.wrap_err("failed to put transfer base fee")?;
4545
Ok(())
4646
}
4747

crates/astria-sequencer/src/accounts/query.rs

+15-14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use anyhow::Context as _;
21
use astria_core::{
32
primitive::v1::{
43
asset,
@@ -9,6 +8,11 @@ use astria_core::{
98
account::v1alpha1::AssetBalance,
109
},
1110
};
11+
use astria_eyre::eyre::{
12+
OptionExt as _,
13+
Result,
14+
WrapErr as _,
15+
};
1216
use cnidarium::{
1317
Snapshot,
1418
StateRead,
@@ -35,19 +39,19 @@ use crate::{
3539
async fn ibc_to_trace<S: StateRead>(
3640
state: S,
3741
asset: asset::IbcPrefixed,
38-
) -> anyhow::Result<asset::TracePrefixed> {
42+
) -> Result<asset::TracePrefixed> {
3943
state
4044
.map_ibc_to_trace_prefixed_asset(asset)
4145
.await
4246
.context("failed to get ibc asset denom")?
43-
.context("asset not found when user has balance of it; this is a bug")
47+
.ok_or_eyre("asset not found when user has balance of it; this is a bug")
4448
}
4549

4650
#[instrument(skip_all, fields(%address))]
4751
async fn get_trace_prefixed_account_balances<S: StateRead>(
4852
state: &S,
4953
address: Address,
50-
) -> anyhow::Result<Vec<AssetBalance>> {
54+
) -> Result<Vec<AssetBalance>> {
5155
let stream = state
5256
.account_asset_balances(address)
5357
.map_ok(|asset_balance| async move {
@@ -150,37 +154,34 @@ pub(crate) async fn nonce_request(
150154
}
151155
}
152156

153-
async fn get_snapshot_and_height(
154-
storage: &Storage,
155-
height: Height,
156-
) -> anyhow::Result<(Snapshot, Height)> {
157+
async fn get_snapshot_and_height(storage: &Storage, height: Height) -> Result<(Snapshot, Height)> {
157158
let snapshot = match height.value() {
158159
0 => storage.latest_snapshot(),
159160
other => {
160161
let version = storage
161162
.latest_snapshot()
162163
.get_storage_version_by_height(other)
163164
.await
164-
.context("failed to get storage version from height")?;
165+
.wrap_err("failed to get storage version from height")?;
165166
storage
166167
.snapshot(version)
167-
.context("failed to get storage at version")?
168+
.ok_or_eyre("failed to get storage at version")?
168169
}
169170
};
170171
let height: Height = snapshot
171172
.get_block_height()
172173
.await
173-
.context("failed to get block height from snapshot")?
174+
.wrap_err("failed to get block height from snapshot")?
174175
.try_into()
175-
.context("internal u64 block height does not fit into tendermint i64 `Height`")?;
176+
.wrap_err("internal u64 block height does not fit into tendermint i64 `Height`")?;
176177
Ok((snapshot, height))
177178
}
178179

179180
async fn preprocess_request(
180181
storage: &Storage,
181182
request: &request::Query,
182183
params: &[(String, String)],
183-
) -> anyhow::Result<(Address, Snapshot, Height), response::Query> {
184+
) -> Result<(Address, Snapshot, Height), response::Query> {
184185
let Some(address) = params
185186
.iter()
186187
.find_map(|(k, v)| (k == "account").then_some(v))
@@ -194,7 +195,7 @@ async fn preprocess_request(
194195
};
195196
let address = address
196197
.parse()
197-
.context("failed to parse argument as address")
198+
.wrap_err("failed to parse argument as address")
198199
.map_err(|err| response::Query {
199200
code: Code::Err(AbciErrorCode::INVALID_PARAMETER.value()),
200201
info: AbciErrorCode::INVALID_PARAMETER.info(),

0 commit comments

Comments
 (0)