Only change maxgas when tx out item scheduled to a new vault

Related to #1556 (closed)

Slash manager already has been versioned to 112 in another PR !2869 (merged)

Give more context about the issue , for example , the outbound that get slashed ,

{
  "type": "security",
  "msg": "missing tx out in=00EE6038958EF665BF9BE8701982620DAC5FF14DD519C9155F7F954CB38BC6DD",
  "id": "3351F86BF40F9D3A619DECA6D57DF7DD99B7FE6C4A57E1B944064877D8CAC07F",
  "chain": "BTC",
  "from": "bc1qs4v7zlmedw3pnvc523urpt2v7jkf62sp042kj0",
  "to": "bc1qtm2lxzpa00xg5d22wpmzk5pf9t73ad4xchvm8x",
  "coin": "3538275 BTC.BTC",
  "memo": "OUT:00EE6038958EF665BF9BE8701982620DAC5FF14DD519C9155F7F954CB38BC6DD"
}

Thornode issue the txout item originally on block height 11224224(https://thornode.ninerealms.com/thorchain/queue/outbound?height=11224224) , outbound vault is thorpub1addwnpepqd2928a5nf3d6jan45xf6ap6dmsj56jq9n6yhh98rpa6wt4shpcmvpm5hs3, max gas is 51000 on block height 11224524(https://thornode.ninerealms.com/thorchain/queue/outbound?height=11224524) , it get rescheduled , outbound vault is the same one thorpub1addwnpepqd2928a5nf3d6jan45xf6ap6dmsj56jq9n6yhh98rpa6wt4shpcmvpm5hs3 max gas is 34500

The reason it get rescheduled to the same vault means there is already observation of the outbound , but doesn't have consensus yet, as you noticed , that max gas is changed , which cause the slash when observation reach consensus

Another slash:

{
  "type": "security",
  "msg": "missing tx out in=6EB909B99F3492BA9AE6EBEE5CA1FB671432B9A27759971E668EC598FE3C2EEE",
  "id": "A04B6C399F5A07481C0710D7F2E4BA0FB6C253EE976C1715370ABAB324969944",
  "chain": "BTC",
  "from": "bc1q2zz4e9k3v2t53gc8zefgv9eujtgul27z0ndp6l",
  "to": "bc1q68aw9vv2h6j7zplqxkdctq9zgzzp8lt5epxtlx",
  "coin": "297025467 BTC.BTC",
  "memo": "OUT:6EB909B99F3492BA9AE6EBEE5CA1FB671432B9A27759971E668EC598FE3C2EEE"
}

Thornode issue the txout item on block height 11225301 (https://thornode.ninerealms.com/thorchain/queue/outbound?height=11225301) , outbound vault is : thorpub1addwnpepq2ujrkurxa93vlssp2gdnxfn4egzdmjxfwe3qpnua5yyvwwj4wk8wadas6e , outbound value 296981619, max gas: 54000 Rescheduled on block height 11225601 (https://thornode.ninerealms.com/thorchain/queue/outbound?height=11225601) , outbound vault is : thorpub1addwnpepqt4wnvxp3utqm4dxt59ksfwc7l3cugw6ytfudzypj47wdk7wf6pc2atnlkt ,outbound value is: 296981619 , max gas: 54000 This reschedule is legit , because TSS didn't sign it out , you can see the vault get changed

Rescheduled on block height 11225901 (https://thornode.ninerealms.com/thorchain/queue/outbound?height=11225901) , outbound vault is : thorpub1addwnpepqt4wnvxp3utqm4dxt59ksfwc7l3cugw6ytfudzypj47wdk7wf6pc2atnlkt , outbound value is : 296981619 , max gas: 42000 This reschedule to the same vault , and max gas get updated , which cause the slash

In this PR , when the txout item rescheduled to the same vault , we don't update anything , not even max gas , thus the outbound match logic should be able to match the outbound correctly to the tx out item

changed the description

approved this merge request

changed milestone to %Release-1.112.0

changed the description

approved this merge request

merged

(Ahh, I see the needsNewVault function, and how the commit moves the update to within its conditional.)

A question: In the module.go EndBlock,
if err := am.mgr.Slasher().LackSigning(ctx, am.mgr); err != nil {
is line 235
and
if err := am.mgr.TxOutStore().EndBlock(ctx, am.mgr); err != nil {
is line 263, later.

LackSigning calls UnSafeAddTxOutItem, which calls addToBlockOut with ctx.BlockHeight(). (That is, setting it for the current height.)

The TxOutStorage EndBlock then afterwards calls GetBlockOut.

	// update the max gas for all outbounds in this block. This can be useful
	// if an outbound transaction was scheduled into the future, and the gas
	// for that blockchain changes in that time span. This avoids the need to
	// reschedule the transaction to Asgard, as well as avoids slash point
	// accural on ygg nodes.
	txOut, err := tos.GetBlockOut(ctx)

Will this still update the MaxGas of the rescheduled outbound even though LackSigning hasn't rescheduled it, or am I misreading?

GetBlockOut:
return tos.keeper.GetTxOut(ctx, ctx.BlockHeight())

addToBlockOut (taking outboundHeight and as an argument, here ctx.BlockHeight()):
return tos.keeper.AppendTxOut(ctx, outboundHeight, item)

AppendTxOut (taking height as an argument, here ctx.BlockHeight()): \

	block, err := k.GetTxOut(ctx, height)
	if err != nil {
		return err
	}
	block.TxArray = append(block.TxArray, item)
	return k.SetTxOut(ctx, block)

mentioned in merge request !2953 (closed)

added pools-swap-lp vaults labels

mentioned in issue #1556 (closed)

mentioned in issue mayachain/mayanode#113 (closed)

mentioned in merge request mayachain/mayanode!305 (merged)