Submitted by RadiantLabs
https://github.com/initia-labs/OPinit/blob/8119243edd00729124410dcd3cde5dc15b8c2fb8/x/opchild/keeper/deposit.go#L61
The L1 deposit function allows a depositor to send a signed payload to be executed atomically with the deposited tokens, via bridge hooks.
These are executed in L2 by routing messages to the default message handler from the apps router:
From the above code, we can see that the sdk.Result returned by the handler() call is discarded. This is incorrect because
the returned sdk.Result is where the events emitted by handler are stored, because in the handler call the EventManager passed with cacheCtx is immediately discarded:
Also by comparison, we can see that another place that similarly handles messages directly (the MsgExecuteMessages handler), does copy events over to the parent context:
As a consequence, events emitted by L2 hooks are discarded and not propagated to the transactions, and cannot be picked up by apps, and most importantly, bots.
In the below PoC we show the worst case scenario, where a withdrawal is initiated through hooks, and because the EventTypeInitiateTokenWithdrawal event is what bridges tokens to L1, tokens are permanently locked in the bridge.
This is a reasonable scenario where a user could, for example, bridge from L1 gas tokens that are necessary to initiate in L2 a withdrawal
of a different token.
The test can be run if added to the OPinit/x/opchild/keeper/msg_server_test.go test file.
Consider adding manual emission of generated events, as done for MsgExecuteMessages.
beer-1 (Initia) confirmed, but disagreed with severity and commented:
LSDan (judge) commented:
3docSec (warden) commented:
LSDan (judge) commented:
