chore: Fix flaky test `forked_node_liquidity_source_notification_mainnet`

[squadgazzz]

The liquidity_source_notification::forked_node_liquidity_source_notification_mainnet test has become flaky(or probably was flaky initially). This needs to be fixed.

[codersharma2001]

Could you share how you’re setting up the forked mainnet node when you run liquidity_source_notification::forked_node_liquidity_source_notification_mainnet? In my environment the driver’s calls to http://127.0.0.1:8545 keep timing out while the order is being created, so I suspect the upstream RPC (or local Anvil configuration) is throttling or otherwise unresponsive. Knowing what RPC provider/flags you use would help me mirror your setup and verify the fix end‑to‑end.

[squadgazzz]

export FORK_URL_MAINNET=rpc_url
cargo test -p e2e liquidity_source_notification::forked_node_liquidity_source_notification_mainnet

You would also need anvil to be installed. Anvil uses the provided RPC to create a fork. I am not sure this is gonna work well with public RPCs.

[squadgazzz]

In any case, it wouldn't make much sense to tackle this as an external contributor. We won't be able to validate your fix properly, since all forked node tests are ignored in CI due to security reasons.

[codersharma2001]

hi @squadgazzz , Just followed up: I added a wait_for_condition around the Liquorice mock so the test now waits until notification_requests is non-empty before asserting, and dropped the request-count log after verifying it’s working. With Anvil forking mainnet at block 23326100 on my machine (FORK_URL_MAINNET=http://127.0.0.1:8545), the test passes consistently.

[codersharma2001]

[codersharma2001]

In any case, it wouldn't make much sense to tackle this as an external contributor. We won't be able to validate your fix properly, since all forked node tests are ignored in CI due to security reasons.

Hey @squadgazzz, thanks again for the pointers. I dug into the flakiness and wanted to share both the fix and exactly how I verified it locally:

What was going wrong -

The driver actually does call the Liquorice notifier, but in the test we were reading notification_requests immediately after the trade. Because the notifier runs on its own async task, we sometimes hit the assertion before the mock received anything. That explains why the test passed intermittently.

What I changed -

In services/crates/e2e/tests/e2e/liquidity_source_notification.rs I wrapped the notification check in our existing wait_for_condition, so we don’t unwrap until notification_requests is non-empty. That removes the race without touching any production code.

How I validated it -

Since forked-node tests don’t run in public CI, I spun them up locally:

1. Started Anvil on port 8546, forking mainnet at block 23326100, and kept it warmed (USDC/USDT contract reads, eth_getBlockByNumber).

2. Ran the test with

FORK_URL_MAINNET=http://127.0.0.1:8546 cargo test -p e2e --test e2e liquidity_source_notification::forked_node_liquidity_source_notification_mainnet -- --ignored --nocapture

3. The harness launched its own Anvil on 8545, but because it forked from my warmed instance, all RPC calls returned instantly and the test passed consistently. I also logged the length of notification_requests during the wait to confirm the mock recorded the notification before we asserted.

This keeps the e2e test aligned with the production flow, so whenever you run the forked suite internally it should be deterministic again. Let me know if you want me to tweak anything else or capture more logs.

[Grinsven]

Wrapped the notification check in wait_for_condition so it waits until notification_requests is non-empty before asserting. Same pattern as the trade check on line 319.

Couldn't test locally with public RPCs since they don't have archive data for block 23326100, but the fix compiles and passes clippy. Would need your internal setup to verify it actually works.

PR: #3819