Skip to content

[V124-specific] Use refundTx for streaming swap partial refunds

Multipartite requested to merge Multi/partial-refund-refundtx into develop

[Midgard note: If this MR is merged, this Midgard line (and its comment) should be updated to behave the same for EventMintBurn Reason "failed_outbound".]

[V124-specific]

Intended to close #1586 (closed)
'Streaming swap partial refunds do not call refundTx'.

As always, suggestions are welcome.

This includes an err -> refundErr correction in reference to these lines (close together in V102):
https://gitlab.com/thorchain/thornode/-/blob/v1.102.0/x/thorchain/manager_swap_queue_current.go#L162-165 \

			ctx.Logger().Error("fail to swap", "msg", pick.msg.Tx.String(), "error", err)
			if newErr := refundTx(ctx, ObservedTx{Tx: pick.msg.Tx}, mgr, CodeSwapFail, err.Error(), ""); nil != newErr {
				ctx.Logger().Error("fail to refund swap", "error", err)
			}

'err' is already logged in the "fail to swap" error log,
so instead of duplicating it in the "fail to refund swap" error log,
that log should use newErr (refundErr in V115) instead.

For V115, though the lines are more separated and newErr is renamed refundErr, but the principle is the same.
https://gitlab.com/thorchain/thornode/-/blob/v1.115.0/x/thorchain/manager_swap_queue_current.go#L198
ctx.Logger().Error("fail to swap", "msg", pick.msg.Tx.String(), "error", err)
https://gitlab.com/thorchain/thornode/-/blob/v1.115.0/x/thorchain/manager_swap_queue_current.go#L235-237 \

				if nil != refundErr {
					ctx.Logger().Error("fail to refund swap", "error", err)
				}

[Update: I preserve the below as a record of my thought process, conclusion at the end.]

I am also currently curious about why the common.BTCAsset outbound in a test here does not have an OUT: memo,
despite being from GetOutboundItems, which refers to GetTxOut,
and addToBlockOut (which calls AppendTxOut which calls SetTxOut)
is called later in cachedTryAddTxOutItem than prepareTxOutItem, which prepares the OUT: memo when the memo is empty.

	// Default the memo to the standard outbound memo
	if toi.Memo == "" {
		toi.Memo = NewOutboundMemo(toi.InHash).String()
	}

[Update:
https://gitlab.com/thorchain/thornode/-/blob/v1.115.0/x/thorchain/manager_swap_queue_current_test.go#L320
mgr.txOutStore = NewTxStoreDummy()
https://gitlab.com/thorchain/thornode/-/blob/v1.115.0/x/thorchain/manager_txout_dummy_test.go#L15-21
NewTxStoreDummy prepares *TxOutStoreDummy.
https://gitlab.com/thorchain/thornode/-/blob/v1.115.0/x/thorchain/manager_txout_dummy_test.go#L47-53
*TxOutStoreDummy-received TryAddTxOutItem calls addToBlockOut with no NewOutboundMemo in a prepareTxOutItem.]

Update:
This also resolves block 11949403's minting of BNB/BTCB synths which should have been refunded from AsgardName,
since the refundTx calls have "" (representing AsgardName) as the source module,
while current code uses ModuleName as the source for both partial successes and partial refunds.
(An alternative resolution is to do minting as well as burning for each sub-swap, then do both transfers from AsgardName;
please note #1521 (closed)/!2843 (merged).)

Current code at time of writing:
https://gitlab.com/thorchain/thornode/-/blob/v1.116.0/x/thorchain/manager_swap_queue_current.go#L303-308

				for _, item := range tois {
					// let the txout manager mint our outbound asset if it is a synthetic asset
					if item.Chain.IsTHORChain() && (item.Coin.Asset.IsSyntheticAsset() || item.Coin.Asset.IsDerivedAsset()) {
						item.ModuleName = ModuleName
					}
					ok, err := mgr.TxOutStore().TryAddTxOutItem(ctx, mgr, item, cosmos.ZeroUint())
Edited by Multipartite

Merge request reports