Fix TestPublishIdentityUpdate test #607
Conversation
WalkthroughThe changes update test scenarios and nonce management logic. In the blockchain publisher tests, the names, context usage, and parameters of two test cases are swapped. In the nonce manager tests, a new heap type is introduced to manage and recycle nonces. The test manager gains an Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NonceManager
participant Heap
Client->>NonceManager: GetNonce(ctx)
alt Context Cancelled?
NonceManager-->>Client: Return error
else
alt Abandoned nonce available?
NonceManager->>Heap: Pop smallest nonce
Heap-->>NonceManager: Return nonce
NonceManager-->>Client: Return recycled nonce
else
NonceManager->>NonceManager: Use current nonce and increment counter
NonceManager-->>Client: Return new nonce
end
end
Client->>NonceManager: Cancel() after use
NonceManager->>Heap: Push nonce back for reuse
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24) ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/blockchain/nonceManager_test.go (2)
15-38: Well-implemented heap interface for nonce managementThe Int64Heap implementation properly implements all the required heap interface methods. This provides an efficient way to store and retrieve abandoned nonces for reuse.
The Peek method is a useful addition that allows checking the smallest element without removing it, though it returns -1 for empty heaps which could be confusing as -1 could be mistaken for a valid value in some contexts.
Consider using a more explicit approach for the empty heap case in Peek():
func (h *Int64Heap) Peek() int64 { if len(*h) == 0 { - return -1 // Return an invalid value if empty + panic("cannot peek an empty heap") // Or return (0, false) if you change the return type } return (*h)[0] }
70-75: Updated nonce cancel function to recycle noncesThe nonce cancellation now properly pushes the nonce back to the abandoned heap, allowing it to be reused later. This completes the nonce recycling mechanism.
The comment "// No-op" at line 75 is no longer accurate since the Cancel function now performs an action.
Remove the outdated comment:
Cancel: func() { tm.mu.Lock() defer tm.mu.Unlock() tm.abandoned.Push(nonce) -}, // No-op +},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/blockchain/blockchainPublisher_test.go(2 hunks)pkg/blockchain/nonceManager_test.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build pre-baked anvil-xmtpd
🔇 Additional comments (6)
pkg/blockchain/blockchainPublisher_test.go (2)
59-64: Test case renamed with correct expectationsThe test case has been properly renamed from "happy path" to "cancelled context" with appropriate changes:
- Using a cancelled context
- Expecting an error to occur
- Reduced size of identityUpdate to 100 bytes
This change aligns the test case name with its actual behavior and expectations.
73-78: Test case renamed with correct expectationsThe test case has been properly renamed from "cancelled context" to "happy path" with appropriate changes:
- Using a background context
- Expecting successful execution (no error)
- Increased size of identityUpdate to 104 bytes
This change correctly represents a successful execution path for the test.
pkg/blockchain/nonceManager_test.go (4)
4-4: Added heap import for nonce recycling mechanismThe container/heap package is correctly imported to support the new nonce recycling functionality.
44-44: Added abandoned nonce trackingThe TestNonceManager now properly tracks abandoned nonces with the Int64Heap field, which enables efficient nonce recycling.
52-54: Added context cancellation checkThis change properly checks if the context is already cancelled before attempting to get a nonce, which prevents unnecessary operations and returns the appropriate error.
59-65: Implemented nonce recycling mechanismThe logic now efficiently reuses nonces from the abandoned heap before generating new ones. This should improve resource usage by preventing nonce values from growing unnecessarily.
The implementation correctly:
- Checks if abandoned nonces are available
- Pops the smallest one if available (maintaining order)
- Falls back to incrementing the nonce counter if none are available
The test nonce manager (which is not backed by a DB) was buggy.
Summary by CodeRabbit
Tests
New Features