Skip to content

[V136-specific] Consistent Halt key height inequalities #check-lint-warning

Multipartite requested to merge Multi/halt-inequality-consistency into develop

[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' V1s.


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
Edited by Multipartite

Merge request reports