Skip to content

Commit

Permalink
feat(sequencer, bridge-withdrawer)!: enforce withdrawals consumed (#1391
Browse files Browse the repository at this point in the history
)

## Summary
Adds state for bridge events and consumes the events as they occur.
Updates proto for bridge unlock to no longer use memo for rollup
information and instead include on main action. Renames
`rollup_transaction_hash` to `rollup_withdrawal_event_id`.

## Background
There was only implicit stop against reusing a withdrawal event, this
adds consumption of withdrawal events. While this is not strictly
security enhancing, consuming the event adds in protocol protection
against accidental double spend by the bridge operator.

## Changes
- Proto updates for `BridgeUnlockAction` + `ICS20WithdrawalMemo`
- Add stateless checks on bridging unlock and withdrawals of correct
information filled out
- Added state to enforce rollup event cannot be consumed twice. 
- Enforce that `Ics20WithdrawalAction` must have `bridge_address` set if
making a bridge withdrawal

## Testing
smoke test + minor test updates

## Breaking Changelist
- New stateful information about rollup withdrawal events
- Adds new fields to `BridgeUnlockAction`

## Related Issues
closes #1430

---------

Co-authored-by: Fraser Hutchison <[email protected]>
Co-authored-by: Richard Janis Goldschmidt <[email protected]>
  • Loading branch information
3 people authored and jbowen93 committed Sep 6, 2024
1 parent 8a9cdf1 commit 09f5505
Show file tree
Hide file tree
Showing 20 changed files with 337 additions and 309 deletions.
16 changes: 6 additions & 10 deletions crates/astria-bridge-contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ where
.ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))?
.as_u64();

let rollup_transaction_hash = log
let rollup_withdrawal_event_id = log
.transaction_hash
.ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))?
.to_string();
Expand All @@ -400,7 +400,7 @@ where
memo: event.memo.clone(),
rollup_block_number,
rollup_return_address: event.sender.to_string(),
rollup_transaction_hash,
rollup_withdrawal_event_id,
})
.map_err(GetWithdrawalActionsError::encode_memo)?;

Expand Down Expand Up @@ -433,20 +433,14 @@ where
.ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))?
.as_u64();

let rollup_transaction_hash = log
let rollup_withdrawal_event_id = log
.transaction_hash
.ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))?
.to_string();

let event = decode_log::<SequencerWithdrawalFilter>(log)
.map_err(GetWithdrawalActionsError::decode_log)?;

let memo = memo_to_json(&memos::v1alpha1::BridgeUnlock {
rollup_block_number,
rollup_transaction_hash,
})
.map_err(GetWithdrawalActionsError::encode_memo)?;

let amount = calculate_amount(&event, self.asset_withdrawal_divisor)
.map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?;

Expand All @@ -456,7 +450,9 @@ where
let action = astria_core::protocol::transaction::v1alpha1::action::BridgeUnlockAction {
to,
amount,
memo,
rollup_block_number,
rollup_withdrawal_event_id,
memo: String::new(),
fee_asset: self.fee_asset.clone(),
bridge_address: self.bridge_address,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,7 @@ fn rollup_height_from_signed_transaction(
.ok_or_eyre("last transaction by the bridge account did not contain a withdrawal action")?;

let last_batch_rollup_height = match withdrawal_action {
Action::BridgeUnlock(action) => {
let memo: memos::v1alpha1::BridgeUnlock = serde_json::from_str(&action.memo)
.wrap_err("failed to parse memo from last transaction by the bridge account")?;
Some(memo.rollup_block_number)
}
Action::BridgeUnlock(action) => Some(action.rollup_block_number),
Action::Ics20Withdrawal(action) => {
let memo: memos::v1alpha1::Ics20WithdrawalFromRollup =
serde_json::from_str(&action.memo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ use astria_core::{
},
protocol::{
bridge::v1alpha1::BridgeAccountLastTxHashResponse,
memos::v1alpha1::{
BridgeUnlock,
Ics20WithdrawalFromRollup,
},
memos::v1alpha1::Ics20WithdrawalFromRollup,
transaction::v1alpha1::{
action::{
BridgeUnlockAction,
Expand Down Expand Up @@ -423,11 +420,9 @@ pub fn make_bridge_unlock_action(receipt: &TransactionReceipt) -> Action {
let inner = BridgeUnlockAction {
to: default_sequencer_address(),
amount: 1_000_000u128,
memo: serde_json::to_string(&BridgeUnlock {
rollup_block_number: receipt.block_number.unwrap().as_u64(),
rollup_transaction_hash: receipt.transaction_hash.to_string(),
})
.unwrap(),
rollup_block_number: receipt.block_number.unwrap().as_u64(),
rollup_withdrawal_event_id: receipt.transaction_hash.to_string(),
memo: String::new(),
fee_asset: denom,
bridge_address: default_bridge_address(),
};
Expand All @@ -448,7 +443,7 @@ pub fn make_ics20_withdrawal_action(receipt: &TransactionReceipt) -> Action {
memo: "nootwashere".to_string(),
rollup_return_address: receipt.from.to_string(),
rollup_block_number: receipt.block_number.unwrap().as_u64(),
rollup_transaction_hash: receipt.transaction_hash.to_string(),
rollup_withdrawal_event_id: receipt.transaction_hash.to_string(),
})
.unwrap(),
fee_asset: denom,
Expand Down
29 changes: 4 additions & 25 deletions crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

139 changes: 13 additions & 126 deletions crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.serde.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 09f5505

Please sign in to comment.