feat(sequencer)!: implement BridgeTransfer action

[noot]

Summary

implement BridgeTransfer action which transfers funds from one bridge account to another. essentially, it atomically performs a BridgeUnlock and a BridgeLock.

Background

this is functionality we want and BridgeUnlocking to another bridge account wouldn't create a Deposit correctly. the issue is that BridgeUnlock does not contain all the information needed to create a deposit event, namely destination_chain_address, so BridgeUnlock directly into another bridge account doesn't work as a lock. BridgeLock from a bridge account to another also doesn't contain the desired withdrawal information (eg rollup_withdrawal_event_id). an atomic BridgeUnlock/BridgeLock combo would only work if the BridgeUnlock unlocks the funds to itself, providing rollup_withdrawal_event_id, and then BridgeLocks to the receiving account. BridgeUnlock cannot go directly into a different account as the bridge would no longer be able to access it. however this seems unwieldly as opposed to just having one action which makes the intention clear.

Changes

• slightly refactor BridgeUnlock to move checks to their own function
• also slightly refactor BridgeLock to move execution and Deposit emission to their own function
• implement BridgeTransfer action using existing BridgeUnlock checks and BridgeLock execution logic.

Testing

unit tests

Changelogs

Changelogs updated.

Breaking Changelist

• a new action was added, which is a breaking sequencer change.

Related Issues

closes #1921

[joroshiba]

Do we need a memo here?

[noot]

not necessarily, do you see any future use for it? i left it because BridgeUnlock has it, but don't think BridgeUnlock necessarily needs it either. removed for now

[SuperFluffy]

Can we have smoke tests for this?

[SuperFluffy]

Request to expand the background section: why this over a bridge-lock followed by bridge-unlock? Execution of our transactions is by definition atomic (because processing of transactions (and their actions) is sequential).

Is the atomicity of bridge-lock-unlock not enough? Does this mean that the only innovation of this feature is a different deposit being emitted? Is there another reason to have this? Could we address this by amending how we emit deposit events?

[noot]

Is the atomicity of bridge-lock-unlock not enough? Does this mean that the only innovation of this feature is a different deposit being emitted? Is there another reason to have this? Could we address this by amending how we emit deposit events?

the issue isn't deposit events. the issue is that BridgeUnlock does not contain all the information needed to create a deposit event, namely destination_chain_address, so BridgeUnlock directly into another bridge account doesn't work as a lock. BridgeLock from a bridge account to another also doesn't contain the desired withdrawal information (eg rollup_withdrawal_event_id). an atomic BridgeUnlock/BridgeLock combo would only work if the BridgeUnlock unlocks the funds to itself, providing rollup_withdrawal_event_id, and then BridgeLocks to the receiving account. BridgeUnlock cannot go directly into a different account as the bridge would no longer be able to access it. however this seems unwieldly as opposed to just having one action which makes the intention clear.

[SuperFluffy]

Is the atomicity of bridge-lock-unlock not enough? Does this mean that the only innovation of this feature is a different deposit being emitted? Is there another reason to have this? Could we address this by amending how we emit deposit events?

the issue isn't deposit events. the issue is that BridgeUnlock does not contain all the information needed to create a deposit event, namely destination_chain_address, so BridgeUnlock directly into another bridge account doesn't work as a lock. BridgeLock from a bridge account to another also doesn't contain the desired withdrawal information (eg rollup_withdrawal_event_id). an atomic BridgeUnlock/BridgeLock combo would only work if the BridgeUnlock unlocks the funds to itself, providing rollup_withdrawal_event_id, and then BridgeLocks to the receiving account. BridgeUnlock cannot go directly into a different account as the bridge would no longer be able to access it. however this seems unwieldly as opposed to just having one action which makes the intention clear.

Please add this to the PR description before merging.

[SuperFluffy]

astria-core needs an entry for this change.

[joroshiba]

approving for snapshot changes + proto changes. One small nit on the validation.

Noting by default this action can't be done as it is disabled because there are no fees, correct?

[joroshiba]

should there also be validation that the destination_chain_address is set, as well as some restrictions on it's length?

Maybe this is more a sequencer concern, notice we don't have this on bridge lock but probably should? There's a limit to what a reasonable sized destination address is.