An unwhitelisted ERC20 token (ETH.RAZE) is in a pool, is being migrated, and is causing MsgSolvency slashes
Update: Closing following !2559 (merged) (unless reopening is requested).
(I may continue to think about whether/how whitelist handling could be more robust.)
Discord context here:
https://discord.com/channels/838986635756044328/839002619481554955/1020662813510869165
Summary:
.yud8 and .6mf9, created during network versions 1.95.0 and 1.95.1 respectively, suffered many ETH chain failed_observe_solvency slashes for not listing ETH.VIU and ETH.RAZE in their solvency messages.
An example (block 7377252):
https://viewblock.io/thorchain/tx/C96CC120DA2CD55D39800B4F8D9807BDC7FD263F4E0F337AA5A060F4C52AF08F
https://viewblock.io/thorchain/tx/613D6F01E6C171EF24453A3D91A385411BF5197AA6447E7D664329FAD32F01AE
https://thornode.ninerealms.com/txs?limit=100&tx.height=7377252&message.action=solvency
Both were present ("VIU" and "RAZE") in the ETH token V93 whitelist and were removed for the ETH token V95 whitelist.
https://gitlab.com/thorchain/thornode/-/blob/v1.95.0/common/tokenlist/ethtokens/eth_mainnet_V93.json
https://gitlab.com/thorchain/thornode/-/blob/v1.95.0/common/tokenlist/ethtokens/eth_mainnet_V95.json
I speculate that they never recorded the Asgard-stored token amounts because their ETHScanner only checked whitelisted tokens.
( https://gitlab.com/thorchain/thornode/-/blob/v1.96.1/bifrost/pkg/chainclients/ethereum/ethereum_block_scanner.go#L162 )
Churns had taken place in blocks 7033385 (2022-08-25) and 2022-09-01 (2022-09-01), but these unwhitelisted tokens were still being migrated
( https://gitlab.com/thorchain/thornode/-/blob/v1.96.1/x/thorchain/manager_network_current.go#L258 )
and were still being reported in solvency messages.
( https://gitlab.com/thorchain/thornode/-/blob/v1.96.1/bifrost/thorclient/thorchain.go#L232 )
If these were the only aspects then I would directly propose adding whitelist checks to the migration and solvency messages.
However, ETH.RAZE specifically still has a pool with a non-zero asset balance (which likewise seems undesirable for an unwhitelisted token).
https://thornode.ninerealms.com/thorchain/pool/ETH.RAZE-0X5EAA69B29F99C84FE5DE8200340B4E9B4AB38EAC?height=7371799
balance_asset "229984228862564"
Whether trying to Ragnarok the pool or (reasonably) treating unwhitelisted tokens as unsafe and destroying the pool directly,
and/or trying to safely(?) burn the tokens so that they do not remain in the ETH Router,
my current knowledge is insufficient to construct a satisfactory proposal, and likewise for implementing a partial resolution without causing pool-related problems.
Further thoughts:
|
If not migrating unwhitelisted tokens, new nodes' MsgSolvency consistency would not be a problem (not require added code).
|
If returning/forgetting/burning unwhitelisted tokens, not migrating them would not require added code.
|
If screening MsgSolvency for whitelisting status, it could be more convenient to screen the coins in chain-specific ReportSolvency rather than in chain-unspecific GetMsgSolvency.
|
If screening coin migration or MsgSolvency for whitelisted status, it could be necessary to specifically allow the gas asset as well.