[V136-specific] Consistent Halt key height inequalities #check-lint-warning
[V136-specific]
Intended to close #2064
'Halt
key height inequalities are inconsistent'.
The #check-lint-warning
is intended to solely be for the keeper_halt.go
functions' V1
s.
There are four things that I currently wish to draw attention to for feedback:
1.
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/bifrost/blockscanner/blockscanner.go#L168-175
// Check if all chains halted globally
globalHaltHeight, err := b.thorchainBridge.GetMimir("HaltChainGlobal")
if err != nil {
b.logger.Error().Err(err).Msg("fail to get mimir setting HaltChainGlobal")
}
if globalHaltHeight > haltHeight {
haltHeight = globalHaltHeight
}
This isChainPaused
logic ignores treats the chain as unpaused when the chain's Halt%sChain
is a past height and Halt%sChain
is a future height.
I currently change this to returning true
after checking each Mimir key if that Mimir key is sufficient to halt the chain,
and without going on to checking other Mimir keys
(since the previous return
used ||
s anyway).
2.
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/keeper/v1/keeper_halt.go#L93-101
PauseLP
and PauseLP%s
despite being named Pause
rather than Halt
act like a Halt
,
namely halting forever after the set height rather than temporarily pausing until the set height.
I request feedback on whether to keep it like this.
3.
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/app/app_default.go#L15-17
It appears appropriate that the HaltHeight
inequality in BeginBlocker
similarly halt in the specified block,
but I wish to ask if there are any undesirable hardfork effects.
For reference, the first hard fork had distinct blocks
https://rpc-v0.ninerealms.com/block?height=4786559
https://rpc-v1.ninerealms.com/block?height=4786560
with the v1 chain starting on a round number, while the second hard fork had two incompatible versions of the same block,
including the v1 chain ending one block after a round number.
https://rpc-v1.ninerealms.com/block?height=17562001
"hash": "2E87B59F3D8C5698EC494E6D6E30C49CF779A977CA8C463AFECD453A892963AB",
https://rpc-v2.ninerealms.com/block?height=17562001
"hash": "0B3C8F9E3EA7E9B1C10CAC5217ED771E0540671EFB9C5315BF01167266BCBEDF",
4.
The below, which I have not touched at this time, are named Pause
yet neither act as an until-block temporary pause
nor a from-block indefinite halt.
Rather, they act as a boolean, seemingly suited to be named Disable
(0 functionality on, 1 functionality off)
or else to be reversed and named Enable
(0 off, 1 on).
I similarly request feedback as to whether to leave them behaving as they are (for now).
PauseBond
:
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/handler_bond.go#L75-77
bondPause := h.mgr.Keeper().GetConfigInt64(ctx, constants.PauseBond)
if bondPause > 0 {
return ErrInternal(err, "bonding has been paused")
PauseUnbond
:
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/handler_unbond.go#L71-72
if h.mgr.Keeper().GetConfigInt64(ctx, constants.PauseUnbond) > 0 {
return ErrInternal(err, "unbonding has been paused")
PauseLoans
:
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/handler_loan_open.go#L71-73
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/handler_loan_repayment.go#L76-78
pauseLoans := h.mgr.Keeper().GetConfigInt64(ctx, constants.PauseLoans)
if pauseLoans > 0 {
return fmt.Errorf("loans are currently paused")
StreamingSwapPause
:
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/manager_swap_queue_current.go#L49-51
pausedStreaming := vm.k.GetConfigInt64(ctx, constants.StreamingSwapPause)
if pausedStreaming > 0 {
continue
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/handler_swap.go#L123-125
pausedStreaming := h.mgr.Keeper().GetConfigInt64(ctx, constants.StreamingSwapPause)
if pausedStreaming > 0 {
return fmt.Errorf("streaming swaps are paused")
PauseAsymWithdrawal-%s
:
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/withdraw.go#L78
pauseAsym, _ := mgr.Keeper().GetMimir(ctx, fmt.Sprintf("PauseAsymWithdrawal-%s", pool.Asset.GetChain()))
https://gitlab.com/thorchain/thornode/-/blob/v2.135.0/x/thorchain/withdraw.go#L173-174
if pauseAsym > 0 {
return common.EmptyAsset