Submitted by RadiantLabs
https://github.com/initia-labs/opinit-bots/blob/640649b97cbfa5782925b7dc8c0b62b8fa5367f6/executor/batchsubmitter/batch.go#L147
https://github.com/initia-labs/opinit-bots/blob/640649b97cbfa5782925b7dc8c0b62b8fa5367f6/executor/batchsubmitter/batch.go#L167
The executors MsgsToProcessedMsgs() converts messages to processed msgs:
Messages are put into batches which will later get broadcasted as a single Cosmos SDK transaction. Additionally, processed messages are persisted to storage to ensure the executor can resume where it left off in case it restarts.
The issue is that the Timestamp (in nanoseconds) in line 291 (and the other instances linked to the submission) is very likely shared between batches, which results in key collisions in the database, as the Key() is based on the Timestamp.
opinit-bots/node/broadcaster/types/db.go::Key()
This leads to wrongly deleting processed messages in the database,
opinit-bots/node/broadcaster/db.go::DeleteProcessedMsgs()
Or, overwriting processed messages:
opinit-bots/node/broadcaster/db.go::SaveProcessedMsgsBatch()
Similarly, when broadcasting a Cosmos transaction to the host/child node, the timestamp is also used as the key for PendingTxInfo when storing it in the db for eventual restarts.
Ultimately, this leads to scenarios where after an executor restart, it will not resume correctly and misses to broadcast critical Cosmos transactions (e.g., finalizing a L1->L2 token deposit).
We prepared a quick PoC that demonstrates how MsgsToProcessedMsgs() constructs batches of messages (ProcessedMsgs) with the same timestamp.
Consider using a truly unique identifier instead of the timestamp to avoid key collisions in the database.
hoon (Initia) commented:
beer-1 (Initia) confirmed and commented:
