Mx 15964 bridge service update #14
Conversation
| @@ -122,9 +122,10 @@ func (ts *txSender) createAndSendTxs(ctx context.Context, data *sovereign.Bridge | |||
| } | |||
| default: | |||
| log.Error("invalid tx data received", "data", string(txData)) | |||
| continue | |||
There was a problem hiding this comment.
I would say that this is a fatal error and should not continue...
Not very sure about it, though...
There was a problem hiding this comment.
This is better than before. Previously we would send the tx with invalid tx data, this is protection to skip it.
There was a problem hiding this comment.
It is better, but I meant we should directly return error, not continue.
This is fatal, we should not skip and send "half txs"
| @@ -127,7 +127,8 @@ func TestTxSender_SendTxs(t *testing.T) { | |||
| }, | |||
| } | |||
| args.TxNonceHandler = &testscommon.TxNonceSenderHandlerMock{ | |||
| ApplyNonceAndGasPriceCalled: func(ctx context.Context, address core.AddressHandler, tx *transaction.FrontendTransaction) error { | |||
| ApplyNonceAndGasPriceCalled: func(ctx context.Context, txs ...*transaction.FrontendTransaction) error { | |||
| require.Len(t, txs, 1) // we update transactions one at a time | |||
There was a problem hiding this comment.
Maybe...
Should we refactor to create all txs and then send them all in bulk?
There was a problem hiding this comment.
I think I tried this and didn't work as expected, not sure if it was my bad or the sdk didn't work properly when spammed.
I'd say we keep it one at the time for now.
There was a problem hiding this comment.
Can we add a task to check if we send them in bulk?
It's better logic anyway... have them all formatted + send them all;
Instead, right now, we can format 2/3 correct + send them, but fail for the third one...
I think it's better to use the ""all or nothing" approach.
Update to use nonce handler v3