Skip to content

[Version-unspecific] Move Trade Account Deposit and Withdraw events from handlers to manager #check-lint-warning

[Version-unspecific]

Intended to close #1962 (closed)
'[Trade Accounts] Add Emit event for Deposit and withdraw'.

(I note that as the swap handler and trade account handlers are internal handlers,
which if returning an error commit no state changes or event emissions.)

Emitting the event in the manager both ensures the event would not be skipped for any future calls elsewhere, and also emits the after-GetSafeShare amount rather than the current-code unrounded/uncapped amount. (That said, this amount is also returned as a cosmos.Uint by Deposit and Withdrawal, so this is an alternative path if requested.)

Feedback is welcome on whether the current common.NoAddress or instead for instance common.NoopAddress is desired for the event asset_address for when withdrawing/depositing in swaps not involving an external-blockchain inbound or outbound.


2024-05-10 update:
To my delight, !3531 (merged) has been merged and this rebased MR now shows the exact proposed event changes!
Thanks to this, I have rewritten this as a [Version-unspecific] code proposal which emits Trade Account events for previous network versions too
(without causing sync failure, at least not for any regression tests).

An earlier commit history which changed event emission only from V133 onwards is here:
https://gitlab.com/thorchain/thornode/-/commits/6b507e62


Taking /blocks/trade-accounts/invalid.json as an example,
I also note that this now has the "trade_account_withdraw" event emitted appropriately before the "scheduled_outbound" event downstream of it (which the withdraw calculations are required beforehand for),
rather than after (chronologically backwards).

another_image

The above shows a withdraw of 100000000, from which a fee of 14000, and then a scheduled outbound of 99986000 (intuitive),
rather than a fee of 14000 and a scheduled outbound of 99986000 and seemingly only after that a withdraw of 100000000.

Edited by Multipartite

Merge request reports