feat: add ordering by nonce flag to get sequential messages #13394
Conversation
|
@wjmelements is there any way to test this , like instead of running it e2e |
cli/state.go
Outdated
| sort.Slice(sortedMsgs, func(i, j int) bool { | ||
| return sortedMsgs[i].Nonce < sortedMsgs[j].Nonce | ||
| }) | ||
| for _, sm := range sortedMsgs { |
There was a problem hiding this comment.
You now have duplicate code here, you could be doing all of this once, probably appending to a slice defined before the loop (with a named struct type would be good), then doing a second loop to print what's been selected; or perhaps just making a printMsg function you can reuse that handles the "cids" thing and the JSONification. But I think that the loop is simple and cheap enough that you could just do it twice and reduce the duplication down to essentially nothing here
DarkLord017
force-pushed
the
feat/order-by-nonce
branch
from
5d90341 to
52ac86d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new --order-by-nonce flag to the state list-msgs CLI command, enabling users to retrieve messages ordered sequentially by nonce when filtering by a 'from' address. This addresses issue #13246 by providing a way to view messages in the order they were intended to be executed.
Key changes:
- Added
--order-by-nonceboolean flag with appropriate usage documentation - Implemented message buffering and sorting logic that orders messages by nonce when the flag is enabled
- The ordering only applies when a 'from' address filter is specified
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
cli/state.go
Outdated
| sort.Slice(orderedMessages, func(i, j int) bool { | ||
| return orderedMessages[i].Nonce < orderedMessages[j].Nonce | ||
| }) |
There was a problem hiding this comment.
The sorting logic is executed after every tipset iteration, leading to O(n log n) sorting operations per tipset. Consider moving the sort operation outside the main loop (after line 935) to sort all collected messages only once, improving performance when processing multiple tipsets.
cli/state.go
Outdated
| } | ||
| fmt.Println(string(b)) | ||
|
|
||
| if cctx.Bool("order-by-nonce") && !froma.Empty() { |
There was a problem hiding this comment.
The condition cctx.Bool("order-by-nonce") && !froma.Empty() is evaluated for every message. Consider checking this once before the loop and storing the result in a boolean variable to avoid repeated context lookups.
cli/state.go
Outdated
| if cctx.Bool("order-by-nonce") && !froma.Empty() { | ||
| sort.Slice(orderedMessages, func(i, j int) bool { | ||
| return orderedMessages[i].Nonce < orderedMessages[j].Nonce | ||
| }) | ||
| for _, om := range orderedMessages { | ||
| fmt.Println(string(om.Msg)) | ||
| } | ||
| orderedMessages = orderedMessages[:0] | ||
| } |
There was a problem hiding this comment.
Messages are sorted and printed after each tipset, which could result in interleaved output from different tipsets and doesn't provide true global ordering by nonce. If the intent is to order all messages across all tipsets, the sorting and printing should occur after the entire loop completes.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| var orderedMessages []struct { | ||
| Nonce uint64 | ||
| Msg []byte | ||
| } |
There was a problem hiding this comment.
[nitpick] The inline anonymous struct makes the code harder to maintain. Consider defining a named type like type OrderedMessage struct { Nonce uint64; Msg []byte } at the package level or before the command definition.
There was a problem hiding this comment.
should i do this @rvagg @wjmelements
Co-authored-by: Copilot <[email protected]>
2 participants
Related Issues
Fixes #13246
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that: