fix eth_newPendingTransactionFilter test #6361

sudo-shashank · 2025-12-17T20:18:58Z

Summary of changes

Changes introduced in this pull request:

fix test

Reference issue to close (if applicable)

Closes #5916

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

Tests
- Enhanced stateful API test framework with improved contract invocation logic and message handling
- Refined test validation and parameter processing for better reliability
Chores
- Updated CI/CD workflow configuration and testing scripts for improved deployment consistency

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-17T20:19:05Z

Walkthrough

The Calibnet API stateful test infrastructure is updated to use a new wallet environment variable and test script that validates arguments and writes a private key file. The Rust test module introduces a contract invocation helper function and refactors message handling logic to fetch the current tipset epoch.

Changes

Cohort / File(s)	Summary
Workflow and Test Script Configuration `.github/workflows/forest.yml`, `scripts/tests/calibnet_api_test_stateful_check.sh`	Workflow now references `CALIBNET_T4_WALLET` environment variable instead of `CALIBNET_WALLET`. Test script adds usage validation, writes private key to `calibnet_t4_wallet.key`, and updates hard-coded addresses (TO_ADDRESS, FROM_ADDRESS) and contract payload values.
Stateful Test Module `src/tool/subcommands/api_cmd/stateful_tests.rs`	Adds imports for `SignedMessage` and `ETH_CHAIN_ID`. Introduces private `invoke_contract` helper that handles CBOR encoding, nonce fetching, message building, gas estimation, signing, and mempool submission. Refactors `eth_newPendingTransactionFilter` test to use the new helper. Updates `wait_pending_message` to fetch current tipset epoch. Removes ignore annotation from test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Rust test logic refactoring: The invoke_contract helper consolidates multiple message-handling steps; verify correct gas estimation, signing, and EIP-1559 transaction construction.
Address and payload updates: Confirm new TO_ADDRESS, FROM_ADDRESS, and calldata/PAYLOAD hex values are correct and match expected contract state.
Tipset epoch handling: Verify that fetching ChainHead and using its epoch in wait_pending_message correctly resolves the timing issue in the pending transaction filter test.

Possibly related PRs

Add forest api test-stateful subcommand #5836: Modifies the same stateful test infrastructure in src/tool/subcommands/api_cmd/stateful_tests.rs and related Calibnet test files—direct code-level overlap.

Suggested labels

RPC

Suggested reviewers

akaladarshi
elmattic
LesnyRumcajs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the specific test being fixed (eth_newPendingTransactionFilter) and matches the primary objective of resolving the failing test in the changeset.
Linked Issues check	✅ Passed	The code changes directly address issue #5916 by fixing the eth_newPendingTransactionFilter test to properly invoke contracts and wait for pending messages, enabling the test to pass without being ignored.
Out of Scope Changes check	✅ Passed	All changes relate to fixing the eth_newPendingTransactionFilter test: Rust code refactors contract invocation logic, shell script updates test addresses and configuration, and workflow updates environment variables. All changes are within scope of the linked issue #5916.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch shashank/pending-tx-filter-test-fix

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

scripts/tests/calibnet_api_test_stateful_check.sh (2)
19-21: Private key file is not cleaned up after use.

The private key is written to calibnet_t4_wallet.key but never removed, leaving sensitive data on disk. Consider cleaning up the file after import.

Additionally, passing "$@" to forest_init will forward the private key argument, but based on the harness.sh snippet, forest_init expects optional --backfill-db arguments, not a private key. This may cause unexpected behavior.
🔎 Suggested fix
 echo "$1" > calibnet_t4_wallet.key
+shift  # Remove private key from positional args before passing to forest_init

 forest_init "$@"

 $FOREST_WALLET_PATH --remote-wallet import calibnet_t4_wallet.key
+rm -f calibnet_t4_wallet.key
33-36: Clarify the comment about the Ethereum address.

The comment on line 35 states this is "the Ethereum address corresponding to the contract f4 address," but the address 0xbf89d559af87e0e3d9848d36aa0dea60a2641476 appears to be the mint recipient (derived from FROM_ADDRESS), not the contract address. Consider updating the comment for clarity.
🔎 Suggested fix
 # cast calldata "mint(address,uint256)" 0xbf89d559af87e0e3d9848d36aa0dea60a2641476 1000000000000000000
 #
-# Note that 0xbf89d559af87e0e3d9848d36aa0dea60a2641476 is the Ethereum address corresponding to the contract f4 address.
+# Note that 0xbf89d559af87e0e3d9848d36aa0dea60a2641476 is the Ethereum address corresponding to FROM_ADDRESS (the mint recipient).
src/tool/subcommands/api_cmd/stateful_tests.rs (1)

571-587: Consider using invoke_contract here for consistency.

The eth_get_filter_logs test has similar inline message construction that could potentially use the new invoke_contract helper for consistency. However, since this test is currently ignored (issue #5917), this refactor can be deferred until that issue is addressed.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 150b367 and 5823775.

📒 Files selected for processing (3)

.github/workflows/forest.yml (1 hunks)
scripts/tests/calibnet_api_test_stateful_check.sh (1 hunks)
src/tool/subcommands/api_cmd/stateful_tests.rs (6 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.

📚 Learning: 2025-08-07T09:37:03.079Z

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: scripts/tests/calibnet_eth_mapping_check.sh:49-49
Timestamp: 2025-08-07T09:37:03.079Z
Learning: In Forest project scripts, `forest_wait_api` (called within `forest_init`) ensures basic RPC service readiness, while `forest_wait_for_healthcheck_ready` performs additional comprehensive checks including DB backfill completion. When using `--backfill-db` flag, basic RPC operations can proceed after `forest_init`, but operations requiring complete DB backfill should wait for `forest_wait_for_healthcheck_ready`.

Applied to files:

.github/workflows/forest.yml

📚 Learning: 2025-08-11T12:42:42.848Z

Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.

Applied to files:

.github/workflows/forest.yml

🧬 Code graph analysis (2)

scripts/tests/calibnet_api_test_stateful_check.sh (2)

scripts/tests/harness.sh (1)

forest_init (107-131)

src/key_management/wallet.rs (1)

import (107-112)

src/tool/subcommands/api_cmd/stateful_tests.rs (1)

src/message/signed_message.rs (1)

new_unchecked (35-37)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: Coverage
GitHub Check: Build forest binaries on Linux AMD64
GitHub Check: tests-release

🔇 Additional comments (6)

.github/workflows/forest.yml (1)

434-438: LGTM! Verify the new secret is configured.

The workflow correctly switches to CALIBNET_T4_WALLET for the stateful API tests, which aligns with the T4/f4 address wallet needed for EVM contract interactions. Ensure the CALIBNET_T4_WALLET secret is configured in the repository settings.

src/tool/subcommands/api_cmd/stateful_tests.rs (5)

4-5: LGTM!

The new imports for SignedMessage and ETH_CHAIN_ID are correctly added to support the new invoke_contract helper function.

298-320: LGTM!

Fetching the current tipset via ChainHead and using its epoch for StateWaitMsg is the correct fix. This ensures the wait operation uses an up-to-date chain height, addressing potential timing issues where a stale epoch could cause the message lookup to fail.

322-360: LGTM! The manual EIP-1559 signing is the key fix.

The invoke_contract helper correctly implements manual signing for f4 (Ethereum-style) addresses:

Builds the message and estimates gas

Constructs an EIP-1559 transaction for Ethereum-compatible signing

Signs via WalletSign using the imported T4 wallet key

Pushes the pre-signed message via MpoolPush

This differs from eth_get_filter_logs which uses MpoolPushMessage (node signs). The manual signing approach is required for f4 addresses where the wallet holds the private key.

514-536: LGTM!

The test correctly uses the new invoke_contract helper and properly tracks the returned CID for verification. The flow is now cleaner and addresses the original issue where EthGetFilterChanges was returning no results.

675-681: LGTM! Test is now enabled.

Removing the .ignore() annotation enables the eth_newPendingTransactionFilter test, which was previously skipped due to issue #5916. The invoke_contract fix using proper EIP-1559 signing for f4 addresses should resolve the underlying issue.

fix eth_new_pending_transaction_filter test

747695f

sudo-shashank changed the title ~~fix eth_newPendingTransactionFilter test~~ fix and enable eth_newPendingTransactionFilter test Dec 17, 2025

fix wait_pending_message

d98e5d1

sudo-shashank changed the title ~~fix and enable eth_newPendingTransactionFilter test~~ fix eth_newPendingTransactionFilter test Dec 17, 2025

sudo-shashank and others added 4 commits December 18, 2025 02:05

Merge branch 'main' into shashank/pending-tx-filter-test-fix

a0c6302

Update workflow

b3e9776

refactor invoke contract

7d6cd53

Merge branch 'main' into shashank/pending-tx-filter-test-fix

5823775

sudo-shashank marked this pull request as ready for review December 18, 2025 09:42

sudo-shashank requested a review from a team as a code owner December 18, 2025 09:42

sudo-shashank requested review from LesnyRumcajs and hanabi1224 and removed request for a team December 18, 2025 09:42

coderabbitai bot reviewed Dec 18, 2025

View reviewed changes

hanabi1224 approved these changes Dec 18, 2025

View reviewed changes

sudo-shashank added this pull request to the merge queue Dec 18, 2025

Merged via the queue into main with commit dbb25b2 Dec 18, 2025
70 of 72 checks passed

sudo-shashank deleted the shashank/pending-tx-filter-test-fix branch December 18, 2025 11:28

coderabbitai bot mentioned this pull request Dec 18, 2025

fix eth_get_filter_logs test #6363

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix eth_newPendingTransactionFilter test #6361

fix eth_newPendingTransactionFilter test #6361

sudo-shashank commented Dec 17, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 17, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix eth_newPendingTransactionFilter test #6361

fix eth_newPendingTransactionFilter test #6361

Conversation

sudo-shashank commented Dec 17, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reference issue to close (if applicable)

Other information and links

Change checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sudo-shashank commented Dec 17, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 17, 2025 •

edited

Loading