Skip to content

Commit

Permalink
feat(sequencer)!: implement bridge sudo and withdrawer addresses (#1142)
Browse files Browse the repository at this point in the history
## Summary
we want bridge accounts to be "upgradable"; ie. we want to be able to
change the key which can control the bridge account without changing the
bridge address from the rollup's point of view. with this change, we
have a sudo key and a withdrawer key for each bridge account, which is
set when the bridge account is initialized. the sudo key is able to
change the sudo key and withdrawer key. the withdrawer key is the only
key allowed to withdraw funds from the bridge account.

## Background
see above

## Changes
- change `InitBridgeAccountAction` to contain a `sudo_address` and
`withdrawer_address` which are set in state
- change `BridgeUnlockAction` to have a `bridge_address` field which is
the bridge address withdrawn from; it also checks if the signer of the
tx is the bridge `withdrawer_address` and fail if not
- implement `BridgeSudoChangeAction` which allows the bridge sudo key to
change the bridge sudo and/or withdrawer keys
- `Ics20Withdrawal`s from bridge now also do a withdrawer key check like
`BridgeUnlockAction`; `bridge_address` was added to the action in the
case that the withdrawer differs from the bridge address
- disallow `Transfer` from a bridge account, as `BridgeUnlockAction` or
`Ics20Withdrawal` must be used.

## Testing
unit tests

## Breaking Changelist
- `InitBridgeAccount` stores two new keys in state: the bridge sudo key
and the bridge withdrawer key
- a new action, `BridgeSudoChangeAction` is added
- this is breaking on the sequencer state machine level.
- however it should NOT be breaking on the protobuf level as nothing was
removed, only new fields/messages were added, and the new fields added
are allowed to be unset. if the fields are unset, it gets converted to a
`None` in the native rust type.
  • Loading branch information
noot authored Jun 8, 2024
1 parent cd5dbd0 commit 29baa40
Show file tree
Hide file tree
Showing 27 changed files with 1,323 additions and 150 deletions.
2 changes: 1 addition & 1 deletion charts/sequencer/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.15.4
version: 0.15.5

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
1 change: 1 addition & 0 deletions charts/sequencer/files/cometbft/config/genesis.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"sequence_byte_cost_multiplier": 1,
"init_bridge_account_base_fee": 48,
"bridge_lock_byte_cost_multiplier": 1,
"bridge_sudo_change_fee": 24,
"ics20_withdrawal_base_fee": 24
},
"allowed_fee_assets": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ fn event_to_bridge_unlock(
))?,
memo: serde_json::to_vec(&memo).wrap_err("failed to serialize memo to json")?,
fee_asset_id,
bridge_address: None,
};

Ok(Action::BridgeUnlock(action))
Expand Down Expand Up @@ -175,6 +176,7 @@ fn event_to_ics20_withdrawal(
source_channel: channel
.parse()
.wrap_err("failed to parse channel from denom")?,
bridge_address: None,
};
Ok(Action::Ics20Withdrawal(action))
}
Expand Down Expand Up @@ -226,6 +228,7 @@ mod tests {
})
.unwrap(),
fee_asset_id: denom.id(),
bridge_address: None,
};

assert_eq!(action, expected_action);
Expand Down Expand Up @@ -265,6 +268,7 @@ mod tests {
})
.unwrap(),
fee_asset_id: denom.id(),
bridge_address: None,
};

assert_eq!(action, expected_action);
Expand Down Expand Up @@ -318,6 +322,7 @@ mod tests {
timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(),
timeout_time: 0, // zero this for testing
source_channel: "channel-0".parse().unwrap(),
bridge_address: None,
};
assert_eq!(action, expected_action);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ fn make_ics20_withdrawal_action() -> Action {
timeout_height: IbcHeight::new(u64::MAX, u64::MAX).unwrap(),
timeout_time: 0, // zero this for testing
source_channel: "channel-0".parse().unwrap(),
bridge_address: None,
};

Action::Ics20Withdrawal(inner)
Expand All @@ -257,6 +258,7 @@ fn make_bridge_unlock_action() -> Action {
})
.unwrap(),
fee_asset_id: denom.id(),
bridge_address: None,
};
Action::BridgeUnlock(inner)
}
Expand Down
2 changes: 2 additions & 0 deletions crates/astria-cli/src/commands/sequencer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ pub(crate) async fn init_bridge_account(args: &InitBridgeAccountArgs) -> eyre::R
rollup_id,
asset_id: default_native_asset_id(),
fee_asset_id: default_native_asset_id(),
sudo_address: None,
withdrawer_address: None,
}),
)
.await
Expand Down

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

Loading

0 comments on commit 29baa40

Please sign in to comment.