Add nonce-fix and replace mpool tools#7084
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds ChangesMempool CLI nonce-fix and replace commands
Sequence DiagramsequenceDiagram
participant User
participant CLI as forest-cli mpool
participant RPC as Filecoin RPC (MpoolPending / ChainNonce)
participant MsgPool as MessagePool (MsgSet / push)
User->>CLI: run `mpool nonce-fix --auto` / `mpool replace --auto`
CLI->>RPC: query actor nonce and MpoolPending
CLI->>CLI: compute fill range / find pending message
CLI->>CLI: compute_replacement_gas() -> compute_rbf_min_premium()
CLI->>RPC: (optional) gas estimation via RPC
CLI->>MsgPool: push signed filler or replacement messages
MsgPool->>RPC: messages propagated to network / mempool state updated
CLI->>User: print new CID / success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docs/users/reference/cli.md`:
- Line 678: Two fenced code blocks in the markdown use bare backticks (``` ) and
need explicit language tags for markdownlint MD040; update the two blocks that
currently start and end with ``` to use ```text as the opening fence so the
examples "Fill an on-chain nonce gap..." and "Replace a pending message in the
mempool..." are fenced with ```text ... ``` instead of just ```, keeping the
content unchanged and only modifying the opening fences to include the language
tag.
In `@src/cli/subcommands/mpool_cmd.rs`:
- Around line 553-564: The Manual branch should validate that gas_feecap >=
gas_premium before calling compute_replacement_gas to avoid late runtime
rejection; add a pre-check where the variables gas_premium and gas_feecap are
available (the Manual branch handling that constructs ReplaceGasInput::Manual)
and return a user-facing error if gas_feecap < gas_premium (e.g. via
context/bail/Err) so the CLI rejects the input immediately rather than letting
compute_replacement_gas or downstream code fail.
- Around line 507-523: When --cid is passed the code currently derives
sender/sequence from ChainGetMessage and then calls find_pending_message(sender,
sequence, &pending), which can match a different pending message if the nonce
was superseded; instead, in the branch where cid is Some(msg_cid) use the
pending list to locate the exact pending entry whose .cid() equals msg_cid (or
otherwise compare the Cid from MpoolPending entries) and use that entry as
original_msg; keep the ChainGetMessage/sequence logic only for the
fallback/no-cid path and ensure find_pending_message is only used when matching
by sender+nonce.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 93adfebf-9a5e-495c-b3ed-1bbebb33f1fe
📒 Files selected for processing (12)
.config/forest.dicCargo.tomldocs/docs/users/reference/cli.mdmise.tomlsrc/cli/subcommands/mpool_cmd.rssrc/message_pool/mod.rssrc/message_pool/msgpool/msg_set.rssrc/message_pool/msgpool/utils.rssrc/rpc/methods/gas.rstests/calibnet_mpool_tools.rstests/calibnet_wallet.rstests/common/calibnet_wallet_helpers.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
39795f8 to
203a8f5
Compare
203a8f5 to
10c1b66
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/subcommands/mpool_cmd.rs`:
- Around line 9-10: The auto RBF logic in compute_replacement_gas uses the
hardcoded REPLACE_BY_FEE_RATIO_DEFAULT when calling compute_rbf, which can
underbid if the node runs a non-default mpool ratio; instead retrieve the
runtime replace_by_fee_ratio from the mpool configuration/API (the same place
the node exposes its mpool settings), pass that value into compute_rbf, and stop
using REPLACE_BY_FEE_RATIO_DEFAULT in compute_replacement_gas (also update the
mirror occurrences around lines 196-207). Ensure you obtain the ratio before
calling compute_rbf and propagate it into cap_gas_fee/compute_rbf calls so the
auto replacement floor matches the running mpool configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ab4093cc-db37-49c2-bc96-000c5361d5e2
📒 Files selected for processing (10)
CHANGELOG.mdCargo.tomldocs/docs/users/reference/cli.mdsrc/cli/subcommands/mpool_cmd.rssrc/message_pool/config.rssrc/message_pool/mod.rssrc/message_pool/msgpool/mod.rssrc/message_pool/msgpool/utils.rssrc/rpc/methods/gas.rstests/common/calibnet_wallet_helpers.rs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
| .with_context(|| format!("no pending message found from {from} with nonce {nonce}")) | ||
| } | ||
|
|
||
| enum ReplaceGasInput { |
There was a problem hiding this comment.
I missed this one earlier sorry, same goes here this is also not needed because it is also just being used to pass to the free functions.
| /// using [`REPLACE_BY_FEE_RATIO_MIN`]. | ||
| /// | ||
| /// See <https://github.com/filecoin-project/lotus/blob/v1.36.0/chain/messagepool/messagepool.go#L210-L213> | ||
| pub(crate) fn compute_rbf_min_premium(premium: &TokenAmount) -> TokenAmount { |
There was a problem hiding this comment.
I would have love to know why these values but I guess lotus also doesn't provide any info on these. If you know please add them.
|
@sudo-shashank Last couple of comments, otherwise LGTM. |
| } | ||
|
|
||
| #[test] | ||
| fn nonce_fix_fill_range_auto() { |
There was a problem hiding this comment.
You can use rstest here to be consistent.
Summary of changes
Changes introduced in this pull request:
nonce-fixandreplacecli commands with test.GasPriceTooLowcondition to check ifgas_premium < min_priceinstead ofgas_premium <= min_price.replace-by-feecalculation to match Lotus, reducing the minimum premium bump for replacement messages from1.25xto1.10x. Ports filecoin-project/lotus#10416.Reference issue to close (if applicable)
Closes #7085
Closes #7086
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
New Features
mpool nonce-fix(auto/manual) andmpool replace(auto/manual RBF) CLI commands.Documentation
nonce-fixandreplaceusage and options.Tests
Chores
Other