There are many Ethereum MsgSolvency messages broadcast because of misinterpreted ETH.RUNE insolvency
!2266 (merged) reads "This PR will double check whether vault is solvent before Sending MsgSolvency , which could reduce chain bloat".
However, looking at Viewblock's transactions list there are frequent Ethereum MsgSolvency transactions with many signers
(so not a single signer's unhealthy blockscanner) and no apparent insolvency.
A recent example (block 7430132, two Asgards only, 84 signers each across all blocks):
https://viewblock.io/thorchain/block/7430132
https://viewblock.io/thorchain/tx/37FB5B1F0A3A4C1836A82730B709183EEF5C07303712AB33697AD61F36DB63DD
The first (.uvcj pub_key here) has no displayed insolvency.
https://viewblock.io/thorchain/tx/71CBF3CD3F05905CF8055EC57FD1EC7FFCE51B6DDC9BB37FC6D99DBA540CA4D0
The second (.mjju pub_key here) has a small ETH.XDEFI insolvency and a small ETH.ETH insolvency. (Note the leeway for gas asset.)
Note that in this block, only those two Asgards list ETH.RUNE . (Would the ETH.DEFI small insolvency normally be ignored by some code?)
https://thornode.ninerealms.com/thorchain/vaults/asgard?height=7430132
|
[Edit: ETH.ETH and ETH.DEFI do not have insolvencies, they have oversolvencies. This appears to be a Viewblock display bug.
Viewblock displays the assets with a red x and 'insolvent', but the lower number is the Asgard vault record and the higher number is the wallet balance (in the MsgSolvency). Tiny oversolvency for Ethereum can be treated as from reasonable rounding down of decimal estimates for caution.
|
Block 7354865 is thus a good example, with all five Asgards listing ETH.RUNE, all having MsgSolvency messages, and the assets of all being oversolvent.]
A less recent example from before v1.91.0 (block 6013579), given that !2339 (merged) Bifrost behaviour (not reporting 0 amounts in MsgSolvency) was implemented in v1.92.0:
Block 6013557, numerous signers, pub_key .d5v9:
https://viewblock.io/thorchain/tx/956E2E4B0E8884616ECAF199BCE0AF10EB05B6A2590A905BEE275519B7FE9DF0
Note that 'ETH.RUNE' is displayed here as 0 and 'solvent', but the .d5v9 Asgard in that block lists 276720000000 (2,767.2) ETH.RUNE,
ETH.RUNE always being burnt immediately upon switching.
Note that handler_solvency.go ignores ETH.RUNE for this reason,
but this handles the solvency message rather than affecting whether the solvency message is broadcast or not.
The crux, now: Bifrost logs show (as an example)
{"level":"info","service":"bifrost","asset":"ETH.RUNE-0X3155BA85D5F96B2D030A4966AF206230E46849CB","asgard amount":"20019165834490","wallet amount":"0","gap":"20019165834490","time":"2022-08-22T12:08:53Z","caller":"/app/bifrost/pkg/chainclients/runners/solvency.go:107","message":"insolvency detected"}
, without any other types of insolvency detection, followed by a MsgSolvency broadcast.
This was for a MsgSolvency for which Viewblock reports a '(-0.00000001)' ETH.THOR insolvency and '(-0.00000006)' ETH.ETH insolvency, but the Bifrost logs there only mention ETH.RUNE and not ETH.THOR or ETH.ETH.
[Edit: See above, ETH.THOR and ETH.ETH had oversolvencies, not insolvencies.]
Relevantly, (as described in #1413 (closed)) new nodes without tokens from the V93 ERC20 whitelist (specifically ETH.RAZE and ETH.VIU)
are quickly accruing slash points from not reaching consensus on whether ETH.RAZE and ETH.VIU balances are observed and reported or not.
A large reason for this is thus the broadcasting of many Ethereum MsgSolvency messages for which the Asgards observed are acceptably solvent,
yet MsgSolvency messages are being triggered by that ETH.RUNE is not being ignored in the IsVaultSolvent function.
I thus propose that code equivalent to (or a corrected version of)
if c.Asset.Equals(common.ERC20RuneAsset()) {
continue
}be added to IsVaultSolvent.
I further suggest that if c.Asset.IsRune() { in handler_solvency.go be changed to if c.Asset.Equals(ERC20RuneAsset()) {
, as IsRune returns 'true' for Asset of type BEP2RuneAsset() as well, which is not burnt upon switching (and it would be alarming if an Asgard thought it had BNB.RUNE which had actually been moved elsewhere to be re-switched).
Other than the node slashes, please keep in mind the chain bloat from these many unnecessary MsgSolvency broadcasts.
Thank you for your time.