Open msgmaxim opened 1 week ago
Attention: Patch coverage is 82.06278%
with 120 lines
in your changes missing coverage. Please review.
Project coverage is 72%. Comparing base (
0e02876
) to head (92d1953
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚨 Try these New Features:
channel_id
is not needed and actually makes no sense outside of Bitcoin, as the premise of Vault swaps is to not need a channel. Therefore passing a channel_id
as a mandatory parameter to vault_swap_request
and arbitrarily hardcoding that to zero for other chains seems misleading to me. It could even be just wrong for chains were channels are reused as we could currently be using that channel.
I'd suggest making that an Option. EDIT: Or if we can fit it into DepositDetails or some other parameter that could make sense.
Pull Request
Closes: PRO-1753
Summary
Added additional fields to
vault_swap_request
(height, deposit_address, channel_id). This is needed for DepositBoosted/DepositFinalised events. I also made it take a vec of deposits (represented asVaultDepositWitness
) instead to be consistent with "regular" deposits and as a better (compared to box-ing individual fields) solution to the "max call size" problem that occurs when we have too many paramters not on a heap. Right now engines still vote on individual deposits (i.e. vec of size 1), but we can change this in the future and submit batches of deposits like we do for regular deposits (@dandanlen mentions that this might require some discussion, so not done here).Added
BoostedVaultTxs
field to track the status of vault deposits (regular deposits useDepositChannelLookup
for this)Added
DepositOrigin
which mimicksSwapOrigin
but excludesInternal
variant and makes more sense in contexts where we are not necessarily dealing with swaps, but deposits in general. I considered merging them into one to simplify, but seemed a bit cleaner to have explicit types (but happy to reconsider).Factored out
process_prewitness_deposit_inner
andprocess_full_witness_deposit_inner
so that the core logic is used regardless of the "origin". Deposit extrinsics now just assemble the parameters for the channel action, perform validation where needed, and read/update the boost status.DepositBoosted/DepositFinalised
events now includeDepositOriginType
so the product knows whetherchannel_id
should be used to lookup deposit channel details for example. @niklasnatterEdit: I might need to check unit-tests/bouncer tests to see why they fail, but otherwise this PR should be read for review.