F/btc delegations #39
Conversation
Remove optional
| /// staking_tx is the staking tx | ||
| pub staking_tx: Bytes, | ||
| /// slashing_tx is the slashing tx | ||
| pub slashing_tx: Bytes, |
There was a problem hiding this comment.
Do we need the full tx or we only need their hash values?
There was a problem hiding this comment.
We only need the hashes, AFAIK. I was thinking about simplifying this as well, but better do it in a different PR, WDYT? Will create an issue for it (if it's not already there).
There was a problem hiding this comment.
Another option is, simplify the entire chain of messages. That is, Babylon sends the hashes already. Though this makes it more dependent on Babylon, and perhaps full validation can't be done. WDYT?
There was a problem hiding this comment.
but better do it in a different PR, WDYT? Will create an issue for it (if it's not already there).
Agree, let's do this incrementally and leave this in subsequent PRs
| /// covenant_sigs is a list of adaptor signatures on the slashing tx | ||
| /// by each covenant member. | ||
| /// It will be a part of the witness for the staking tx output. | ||
| pub covenant_sigs: Vec<CovenantAdaptorSignatures>, |
There was a problem hiding this comment.
We probably don't need covenant signatures anymore after verifying them
There was a problem hiding this comment.
Let's remove them in a different PR then. Can you document all that can be removed / simplified? Will link the issue here.
Update: This is being handled in or as part of #17. Feel free to expand the list under #17 (comment).
| /// staking_output_idx is the index of the staking output in the staking tx | ||
| pub staking_output_idx: u32, | ||
| /// unbonding_time is used in unbonding output time-lock path and in slashing transactions | ||
| /// change outputs | ||
| pub unbonding_time: u32, | ||
| /// undelegation_info is the undelegation info of this delegation. | ||
| pub undelegation_info: BtcUndelegationInfo, | ||
| /// params version used to validate the delegation | ||
| pub params_version: u32, |
There was a problem hiding this comment.
Probably these fields can be consolidated as well? at least staking_output_idx, unbonding_time is not used after verification.
One feature that might be needed is to handle params_version properly. This is the version of BTC staking protocol parameters in Babylon. When Babylon changes the parameters the new parameters will be under an incremented version. Probably Babylon needs to report this to all consumer chains upon updating parameters. This is certainly another feature and needs to be done in another PR
There was a problem hiding this comment.
Let's create one (or two) issues for this.
| } | ||
|
|
||
| #[cw_serde] | ||
| pub struct BtcUndelegationInfo { |
There was a problem hiding this comment.
My current impression is that contract side only needs to remember the unbonding tx hash and unbonding slashing tx hash and that's it
There was a problem hiding this comment.
The delegator_unbonding_sig field is being used to mark the delegation as undelegated. The same can be done through a boolean though.
Use our own structs for handling
btc-stakingstate. This is needed so that we can add fields to these underlying structs. This also removes the use ofBinaryfrom state structs (related to #17).Now we have our own
state::stakingstructs:BtcDelegation(converted frombtc_staking_api::ActiveBtcDelegation).BtcUndelegationInfo(converted frombtc_staking_api::BtcUndelegationInfo).CovenantAdaptorSignatures(converted frombtc_staking_api::CovenantAdaptorSignatures).SignatureInfo(converted frombtc_staking_api::SignatureInfo)This PR also removes optional from
btc_undelegation, for simplicity / clarity.Take into account that these changes are API-breaking for the delegations queries. Likely the structs on the Go side will need to be adapted as well.