Integrate with batch transactions #24
Conversation
cmd/process.go
Outdated
| batchPosition = 0 | ||
| State.Store(dequeuedMsg.SourceTxHash, []*types.MessageState{dequeuedMsg}) | ||
| msg, _ = State.Load(dequeuedMsg.SourceTxHash) | ||
| p.Mu.Lock() | ||
| msg[batchPosition].Status = types.Created | ||
| p.Mu.Unlock() |
There was a problem hiding this comment.
why does the MessageState need to be stored, then loaded before mutating the msg at index 0 to set Status = types.Created?
couldn't you set Status = types.Created before calling Store?
cmd/process.go
Outdated
| if s == dequeuedMsg { | ||
| batchPosition = pos | ||
| found = true | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
it looks like this if statement is checking the equality of two pointer types which may not have the intended behavior here. see: https://stackoverflow.com/questions/34276205/comparing-pointers-in-go
dequeuedMsg is read from the processingQueue channel, which i'm guessing is reading data from over the network, and s is loaded from state so even if the two underlying structs have the same data fields the pointers are pointing to different locations in memory.
to properly check the equality of two different instances of MessageState you will want to make use of an Equals method which explicitly checks the equality of each field in the structs.
cmd/process.go
Outdated
| p.Mu.Lock() | ||
| msg[batchPosition].Status = types.Pending | ||
| msg[batchPosition].Updated = time.Now() | ||
| p.Mu.Unlock() | ||
| time.Sleep(10 * time.Second) |
There was a problem hiding this comment.
out of curiosity why do we need to wait 10 seconds before sending the msg over the processingQueue channel?
cmd/process_test.go
Outdated
| p.Mu.RLock() | ||
| require.Equal(t, types.Created, actualState[0].Status) | ||
| p.Mu.RUnlock() |
There was a problem hiding this comment.
why is the lock necessary in this test case?
cmd/process_test.go
Outdated
| p.Mu.RLock() | ||
| require.Equal(t, types.Complete, actualState[0].Status) | ||
| p.Mu.RUnlock() |
There was a problem hiding this comment.
see previous question about locks being used in these test cases
| SourceTxHash: txHash, | ||
| IrisLookupId: "123", | ||
| Status: Filtered, | ||
| MsgSentBytes: []byte("i like turtles"), |
boojamya
force-pushed
the
dan/batch-tx
branch
from
340b0d0 to
9eb062a
Compare
Batch tx example: https://etherscan.io/tx/0x55dcde141614c0baf34827d1dcdced183b2ea166cec0b849a5c8eb029ec0bb92
Prior to this PR, the relayer couldn't handle batch transactions.
As you can see here:
https://github.com/strangelove-ventures/noble-cctp-relayer/blob/bd7b450e96529bfa75f8e2274591424fa94d11c3/cmd/process.go#L92-L94
We were storing and loading messages by a lookup key created by the
SourceTxHashand theType, however batch transactions could have the sameSourceTxHashand theTypebut different messageMsgSendBytes.So as a transactions with multiple messages would come in, they would overwrite each other in the
processingQueue