Update loading next sequence map to avoid startup failure #11307
Conversation
amit-momin
commented
Nov 15, 2023
amit-momin
commented
- Updated loading next sequence map logic during Broadcaster startup to not return an error. This allows the Broadcaster to not fail startup due to something like an RPC call failure.
- Updated the sequence syncer to load a sequence from the tx table or on-chain if it is not found in the map. This allows the Broadcaster to populate the next sequence map for relevant addresses if it was not during startup.
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
amit-momin
force-pushed
the
bugfix/load-next-seq-map-failure
branch
2 times, most recently
from
1d1b496 to
649ef08
Compare
amit-momin
changed the title
common/txmgr/broadcaster.go
Outdated
| @@ -397,6 +401,14 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) Sync | |||
| // Address not found in map so skip sync | |||
| if err != nil { | |||
| eb.logger.Criticalw("Failed to retrieve local next sequence for address", "address", addr.String(), "err", err) | |||
| // Try to retrieve next sequence from on-chain to load the map | |||
There was a problem hiding this comment.
The problem is that SyncSequence is only called if that option was set to true for broadcaster.
So if a particular fromAddress couldn't sync a sequence at initialization time, then it will never get a sequence.
There was a problem hiding this comment.
I would suggest that use the method GetNextSequence(address ADDR)
Inside there, if you don't find the seq from the map, then directly call getSequenceForAddr(), and also set it in the map.
That will ensure all addresses can sync and get seq if they missed doing it at init time.
There was a problem hiding this comment.
The logic has been moved to GetNextSequence in the latest commit.
amit-momin
changed the title
amit-momin
force-pushed
the
bugfix/load-next-seq-map-failure
branch
from
8f1eb8c to
8fa1c7f
Compare
common/txmgr/broadcaster.go
Outdated
| eb.sequenceLock.Lock() | ||
| defer eb.sequenceLock.Unlock() | ||
| // Get next sequence from map | ||
| seq, exists := eb.nextSequenceMap[address] | ||
| if !exists { | ||
| return seq, errors.New(fmt.Sprint("address not found in next sequence map: ", address)) | ||
| eb.logger.Criticalw("address not found in local next sequence map. Attempting to search and populate sequence.", "address", address.String()) |
There was a problem hiding this comment.
Doesn't need to be a critical error. It just means that we couldn't fetch sequence from onchain due to an unavailable RPC.
Maybe just an info log.
There was a problem hiding this comment.
Will do! It does also mean that we can't broadcast any transaction for that address since we don't have a nonce for it
amit-momin
force-pushed
the
bugfix/load-next-seq-map-failure
branch
2 times, most recently
from
a628bc8 to
5bf5aae
Compare
amit-momin
force-pushed
the
bugfix/load-next-seq-map-failure
branch
from
5bf5aae to
103d05d
Compare
|
SonarQube Quality Gate
|
* Updated loading next sequence map to avoid startup failure * Moved logic to populate next sequence map during runtime * Added changelog * Addressed feedback * Addressed feedback
* Updated loading next sequence map to avoid startup failure * Moved logic to populate next sequence map during runtime * Added changelog * Addressed feedback * Addressed feedback
