-
Notifications
You must be signed in to change notification settings - Fork 2
Sync #153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync #153
Changes from 4 commits
5580e09
7abac3c
87cf58a
2c78a04
fe90e2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ import ( | |
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "regexp" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
@@ -21,6 +23,20 @@ import ( | |
|
|
||
| // Rate limiter defaults are provided by config.DefaultRateLimitPerTokenMs and config.DefaultRateLimitBurst | ||
|
|
||
| // extractExpectedSequence extracts the expected sequence number from an account sequence mismatch error message. | ||
| // It looks for the pattern "expected <number>" in the error message, allowing for optional whitespace. | ||
| // Returns the expected sequence number and true if found, or 0 and false if not found. | ||
| func extractExpectedSequence(errorMsg string) (uint64, bool) { | ||
| re := regexp.MustCompile(`expected\s+(\d+)`) | ||
| matches := re.FindStringSubmatch(errorMsg) | ||
| if len(matches) > 1 { | ||
| if expectedSeq, parseErr := strconv.ParseUint(matches[1], 10, 64); parseErr == nil { | ||
| return expectedSeq, true | ||
| } | ||
| } | ||
| return 0, false | ||
| } | ||
|
|
||
| func calculateTransactionSize(messages []types.Msg) (int64, error) { | ||
| if len(messages) == 0 { | ||
| return 0, nil | ||
|
|
@@ -127,8 +143,8 @@ func (q *Queue) Listen() { | |
|
|
||
| log.Info().Msg("Queue module started") | ||
| for q.running { | ||
| time.Sleep(time.Millisecond * 100) // pauses for one third of a second | ||
| if !q.processed.Add(time.Second * time.Duration(q.interval)).Before(time.Now()) { // minimum wait for 2 seconds | ||
| time.Sleep(time.Millisecond * 100) | ||
| if !q.processed.Add(time.Second * time.Duration(q.interval)).Before(time.Now()) { | ||
| continue | ||
| } | ||
|
|
||
|
|
@@ -145,13 +161,6 @@ func (q *Queue) Listen() { | |
| continue | ||
| } | ||
|
|
||
| // bunch into 25 message chunks if possible | ||
| if total < 25 { // if total is less than 25 messages, and it's been less than 10 minutes passed, skip | ||
| if q.processed.Add(time.Minute * 10).After(time.Now()) { | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| _, _ = q.BroadcastPending() | ||
| q.processed = time.Now() | ||
| } | ||
|
|
@@ -222,7 +231,7 @@ func (q *Queue) BroadcastPending() (int, error) { | |
| var i int | ||
| for !complete && i < 10 { | ||
| i++ | ||
| res, err = q.wallet.BroadcastTxCommit(data) | ||
| res, err = q.wallet.BroadcastTxSync(data) | ||
| if err != nil { | ||
| if strings.Contains(err.Error(), "tx already exists in cache") { | ||
| log.Info().Msg("TX already exists in mempool, we're going to skip it.") | ||
|
|
@@ -234,13 +243,29 @@ func (q *Queue) BroadcastPending() (int, error) { | |
| q.messages = make([]*Message, 0) | ||
| return 0, nil | ||
| } | ||
| if strings.Contains(err.Error(), "account sequence mismatch") { | ||
| if expectedSeq, found := extractExpectedSequence(err.Error()); found { | ||
| data = data.WithSequence(expectedSeq) | ||
| continue | ||
| } | ||
| // Fallback to incrementing if extraction fails | ||
| if data.Sequence != nil { | ||
| data = data.WithSequence(*data.Sequence + 1) | ||
| continue | ||
| } | ||
| } | ||
|
Comment on lines
+246
to
+256
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider extracting duplicated sequence mismatch handling logic. The sequence mismatch handling logic is duplicated between the error path (lines 246-256) and the response path (lines 263-273). This reduces maintainability and increases the risk of inconsistencies if the logic needs to be updated. Consider refactoring into a helper function: // handleSequenceMismatch attempts to recover from a sequence mismatch by extracting
// the expected sequence from the error message or incrementing the current sequence.
// Returns updated TransactionData and true if recovery was attempted, or original data and false otherwise.
func handleSequenceMismatch(data walletTypes.TransactionData, errorMsg string) (walletTypes.TransactionData, bool) {
if expectedSeq, found := extractExpectedSequence(errorMsg); found {
return data.WithSequence(expectedSeq), true
}
// Fallback to incrementing if extraction fails
if data.Sequence != nil {
return data.WithSequence(*data.Sequence + 1), true
}
return data, false
}Then use it in both places: // In error path:
if strings.Contains(err.Error(), "account sequence mismatch") {
if newData, handled := handleSequenceMismatch(data, err.Error()); handled {
data = newData
continue
}
}
// In response path:
if strings.Contains(res.RawLog, "account sequence mismatch") {
if newData, handled := handleSequenceMismatch(data, res.RawLog); handled {
data = newData
continue
}
}Also applies to: 263-273 |
||
| log.Warn().Err(err).Msg("tx broadcast failed from queue") | ||
| continue | ||
| } | ||
|
|
||
| if res != nil { | ||
| if res.Code != 0 { | ||
| if strings.Contains(res.RawLog, "account sequence mismatch") { | ||
| if expectedSeq, found := extractExpectedSequence(res.RawLog); found { | ||
| data = data.WithSequence(expectedSeq) | ||
| continue | ||
| } | ||
| // Fallback to incrementing if extraction fails | ||
| if data.Sequence != nil { | ||
| data = data.WithSequence(*data.Sequence + 1) | ||
| continue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| package queue | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestExtractExpectedSequence(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| errorMsg string | ||
| expectedSeq uint64 | ||
| expectedFound bool | ||
| }{ | ||
| { | ||
| name: "valid error message with expected sequence", | ||
| errorMsg: "error while simulating tx: rpc error: code = Unknown desc = account sequence mismatch, expected 1471614, got 1471613: incorrect account sequence", | ||
| expectedSeq: 1471614, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message from log example", | ||
| errorMsg: "account sequence mismatch, expected 1471614, got 1471613: incorrect account sequence [cosmos/cosmos-sdk@v0.45.17/x/auth/ante/sigverify.go:264] With gas wanted: '0' and gas used: '5043313'", | ||
| expectedSeq: 1471614, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message with different sequence numbers", | ||
| errorMsg: "account sequence mismatch, expected 999999, got 999998", | ||
| expectedSeq: 999999, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message with zero sequence", | ||
| errorMsg: "account sequence mismatch, expected 0, got 1", | ||
| expectedSeq: 0, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message with large sequence number", | ||
| errorMsg: "account sequence mismatch, expected 18446744073709551615, got 18446744073709551614", | ||
| expectedSeq: 18446744073709551615, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message without expected keyword", | ||
| errorMsg: "account sequence mismatch, got 1471613: incorrect account sequence", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| { | ||
| name: "error message with account sequence mismatch but no numbers", | ||
| errorMsg: "account sequence mismatch: incorrect account sequence", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| { | ||
| name: "empty error message", | ||
| errorMsg: "", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| { | ||
| name: "error message with unrelated expected keyword", | ||
| errorMsg: "something went wrong, expected something else", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| { | ||
| name: "error message with expected but no number", | ||
| errorMsg: "account sequence mismatch, expected abc, got 1471613", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| { | ||
| name: "error message with multiple expected keywords", | ||
| errorMsg: "account sequence mismatch, expected 1471614, got 1471613, but expected something else", | ||
| expectedSeq: 1471614, | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message with sequence in different format (commas)", | ||
| errorMsg: "account sequence mismatch, expected 1,471,614, got 1,471,613", | ||
| expectedSeq: 1, // Regex will match the first digit sequence "1" | ||
| expectedFound: true, | ||
| }, | ||
| { | ||
| name: "error message with negative number (should not match)", | ||
| errorMsg: "account sequence mismatch, expected -1471614, got 1471613", | ||
| expectedSeq: 0, | ||
| expectedFound: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| seq, found := extractExpectedSequence(tt.errorMsg) | ||
| require.Equal(t, tt.expectedFound, found, "found flag should match") | ||
| if tt.expectedFound { | ||
| assert.Equal(t, tt.expectedSeq, seq, "sequence number should match") | ||
| } else { | ||
| assert.Equal(t, uint64(0), seq, "sequence should be 0 when not found") | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestExtractExpectedSequence_EdgeCases(t *testing.T) { | ||
| t.Run("very long error message", func(t *testing.T) { | ||
| longMsg := "error while simulating tx: rpc error: code = Unknown desc = account sequence mismatch, expected 1234567890, got 1234567889: incorrect account sequence [cosmos/cosmos-sdk@v0.45.17/x/auth/ante/sigverify.go:264] With gas wanted: '0' and gas used: '5043313' and some other very long text that goes on and on" | ||
| seq, found := extractExpectedSequence(longMsg) | ||
| require.True(t, found) | ||
| assert.Equal(t, uint64(1234567890), seq) | ||
| }) | ||
|
|
||
| t.Run("error message with whitespace", func(t *testing.T) { | ||
| msg := "account sequence mismatch, expected 1471614 , got 1471613" | ||
| seq, found := extractExpectedSequence(msg) | ||
| require.True(t, found) | ||
| assert.Equal(t, uint64(1471614), seq) | ||
| }) | ||
|
|
||
| t.Run("error message with newlines", func(t *testing.T) { | ||
| msg := "account sequence mismatch,\nexpected 1471614,\ngot 1471613" | ||
| seq, found := extractExpectedSequence(msg) | ||
| require.True(t, found) | ||
| assert.Equal(t, uint64(1471614), seq) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Optimize regex compilation for better performance.
The regex is compiled on every function call, which is inefficient. Consider moving the compilation to a package-level variable.
Apply this diff to optimize the regex compilation:
📝 Committable suggestion
🤖 Prompt for AI Agents