analyzer/runtime: Support WROSE events Deposit, Withdrawal

[mitjat]

analyzer/runtime: Support WROSE events Deposit, Withdrawal

This PR adds support for two events specific to the WROSE contract (which is a copy of WETH): Deposit and Withdrawal.

The contract is a valid ERC-20 token, but it also generates these special events when converting between ROSE and WROSE.

Background
A user reported that nexus was reporting an incorrect balance of WROSE for them. Also, after they wrapped and subsequently unwrapped 1 ROSE as an experiment, the nexus balance never updated.
We traced this back to the fact that native token (un)wrapping requires special-casing: we need to recognize those events and at least query the node, but ideally also dead reckon the balances.
For details, see the larger comment blocks in the new code.

Addresses etc from the bug report

User wrapped 1 ROSE in mainnet Emerald block 7847770 (txs, events), then unwrapped in 7847845 (txs, events)

User addr:
  0x2435ff763095d7c8ABfc1F05d1C4031B44013914
  oasis1qrrc6m8ulfcd00fct2jkedptkpc3gp7yzvhs30n5
  AAAAAAAAAAAAAAAAJDX/djCV18ir/B8F0cQDG0QBORQ= (as EVM log topic)
WROSE addr:
  0x21C718C22D52d0F3a789b752D4c2fD5908a8A733
  oasis1qpgcp5jzlgk4hcenaj2x82rqk8rrve2keyuc8aaf
Deposit signature:
  0xe1fffcc4923d04b559f4d29a8bfc6cda04eb5b0d3c460751c2402c5c5cc9109c
Withdrawal signature:
  0x7fcf532c15f0a6db0bd6d0e038bea71d30d808c7d98cb3bf7268a95bf5081b65
  f89TLBXwptsL1tDgOL6nHTDYCMfZjLO/cmipW/UIG2U= (as EVM log topic)

Testing

1. e2e regressions. I verified the diff manually: The new WROSE balances are correct at round 1_004_297 (where regression test stops).
2. I also ran the analyzer over the range of blocks around the above bug report, and confirmed that both ROSE and WROSE get dead-reckoned as expected.

[ptrus]

Just for my understanding: Don't we already dead-reckon the native token for caller and callee on every evm.Call? e.g. via:

nexus/analyzer/runtime/extract.go

Lines 531 to 548 in a1ba62c

	if txr.Result.Ok != nil {
	// Dead-reckon native token balances.
	// Native token transfers do not generate events. Theoretically, any call can change the balance of any account,
	// and we do not have a good way of tracking them; we just query them with the evm_token_balances analyzer.
	// But heuristically, a call is most likely to change the balances of the sender and the receiver, so we create
	// a (quite possibly incorrect) dead-reckoned change of 0 for those accounts, which will cause the evm_token_balances analyzer
	// to re-query their real balance.
	reckonedAmount := amount.ToBigInt() // Calls with an empty body represent a transfer of the native token.
	if len(body.Data) != 0 || len(blockTransactionData.SignerData) > 1 {
	// Calls with a non-empty body have no standard impact on native balance. Better to dead-reckon a 0 change (and keep stale balances)
	// than to reckon a wrong change (and have a "random" incorrect balance until it is re-queried).
	reckonedAmount = big.NewInt(0)
	}
	registerTokenIncrease(blockData.TokenBalanceChanges, evm.NativeRuntimeTokenAddress, to, reckonedAmount)
	for _, signer := range blockTransactionData.SignerData {
	registerTokenDecrease(blockData.TokenBalanceChanges, evm.NativeRuntimeTokenAddress, signer.Address, reckonedAmount)
	}
	}

so this would just need to register the additional WROSE token increases/decreases.
or am I missing something?

[mitjat]

Well spotted. However, there's a subtlety: We dead-reckone native ROSE for every evm.Call that has an empty body (line 539). The call to the withdraw() method of WROSE, for example, is not like that. See it here. The real value (1 ROSE) is hidden in the data field (which contains the signature of the withdraw() method (2e1a7d4d), and 1e18, which is how many wei we're withdrawing).

We could skip dead reckoning of ROSE in the new code, because the dead reckoning (by a delta=0) in the code you linked would cause the evm_token_balances analyzer to later fetch the true balance from the runtime. But with explicit dead reckoning (by the correct amount), the UX should be a little better.

[pro-wh]

problem here is deposit can be empty or non-empty body. anyone want to disable the old reckoning? in my recollection it was a hack we added to support wrose

[ptrus]

for every evm.Call that has an empty body

Yeah that makes sense, since that is almost always a native token transfer.

The call to the withdraw() method of WROSE, for example, is not like that...

Makes sense that withdraw has non-empty body 👍

---

Should we update it so that for every evm.Call we dead-reckon native token for both the caller and callee? Or would that be too much? Otherwise we'd need such special cases for every contract that interacts with the native token? (although maybe WROSE will be the only such contract used in the wild).

[mitjat]

@pro-wh the "hack" is not just for WROSE; any contract can (and multiple do) transfer ROSE under the hood.

@ptrus We do always enqueue both the tx signer and the smart contract account to be re-queried for ROSE balance; see lines 544-547.

The only difference between empty vs nonempty Call::data is just this: If data is nonempty empty, we assume it's too hard to guess know what the contract will do, so we just enqueue the accounts to be re-queried, but we do not dead-reckon any changes. OTOH if data is empty, we assume that Call::value attribute is the amount of ROSE that the call will transfer, and we optimistically use that value for dead-reckoning (but still enqueue the accounts to be re-queried).

So the ROSE balance was never off for long in the WROSE::deposit() case that this PR addresses; we did already enqueue the user's account to be re-queried. But the WROSE balance we did not query; there was no reason to, because the WROSE contract was not (from the perspective of observing just the Deposit or Withdrawal event or tx) an ERC-20.

This gives me the idea to try and enqueue every (contract, account) pair (where account calls contract) to be queried for account's balance of the contract EVM token. I've been playing with that and it enqueues a whole bunch, but IDK yet if it's meaningful. Querying the node for EVM balances gives me HTTP 502 a lot of the time.

[ptrus]

So the ROSE balance was never off for long in the WROSE::deposit() case that this PR addresses; we did already enqueue the user's account to be re-queried

Ok, yeah that's what I was wondering. So only the token WROSE was problematic. I think I actually misread the ticked description initially, I understood that the ROSE balance was wrong as well, which didn't make sense to me.

[pro-wh]

@pro-wh the "hack" is not just for WROSE; any contract can (and multiple do) transfer ROSE under the hood.

contracts that transfer ROSE are the cases that the hack doesn't support. the hack assumes the ROSE goes from the tx sender to the contract and stays there, which supports the wROSE deposit case

[mitjat]

@pro-wh we do query the ROSE balance of the sender and the contract after the evm.Call. But it's true that if the contract forwards the ROSE to some 3rd party address (e.g. one encoded somewhere in the Call body), we won't detect that :(

I ran the experiment above (with enqueuing every evm.Call (contract, account) pair for re-querying the balance of contract token). No ERC20 balances were corrected in the e2e regression test block range(*). So I'll not include that enqueuing for now. I'm leaving the commit that achieves that (+ the commit's immediate revert) in this PR for posterity though).

(*) I think it ran through all the enqueued pairs, though logs are full of 502 errors and a little funky.

[pro-wh]

No ERC20 balances were corrected in the e2e regression test block range(*). So I'll not include that enqueuing for now.

wait I thought we were talking about native balances