Handle lock time change output #86
Conversation
| @@ -739,14 +739,14 @@ func (bc *BabylonController) QueryDelegationInfo(stakingTxHash *chainhash.Hash) | |||
| return retry.Unrecoverable(fmt.Errorf("malformed unbonding transaction: %s: %w", err.Error(), ErrInvalidValueReceivedFromBabylonNode)) | |||
| } | |||
|
|
|||
| if resp.UndelegationInfo.UnbondingTime > math.MaxUint16 { | |||
| return retry.Unrecoverable(fmt.Errorf("malformed unbonding time: %d: %w", resp.UndelegationInfo.UnbondingTime, ErrInvalidValueReceivedFromBabylonNode)) | |||
| if resp.UnbondingTime > math.MaxUint16 { | |||
There was a problem hiding this comment.
Should we also check if unbonding time is bigger than finalisation timeout?
There was a problem hiding this comment.
so this is a bit tough question 😅
If we are connected to full babylon node which we trust (lets say we run it ourselves), in theory we should accept any values it sends our way as it means that whats protocol accepted. This means that even if we would get bitcoin transaction which is malformed but Babylon node accepted it, we should also accept it as current state of the world. Thats why staker do only minimal checks of received values to avoid panics. At this stage of development, those checks are mostly to indicate that something weird happened on babylon. So in theory we could add this check, but it would be only debugging helper to see if something weird is happening on Babylon.
If we are connected to node which we do not trust then checking if unbonding time is bigger than finalisation timeout, does not help us with anything as untrusted node can lie about both values.
So in conclusion, I would avoid this check as it would be at most debugging helper. In general, probably it would be good idea to have config option which indicate whether we are connecting to trusted node or not and based on this have some additional checks.
Note:
This pr does not handle proper recovery of funds after slashing happens and staker need to recover btc from locked output.