Remove ConnectionCache::new_quic_for_tests

[alexpyattaev]

Problem

ConnectionCache::new_quic_for_tests is used in a variety of unit tests across Agave codebase. ConnectionCache has multiple unfortunate design decisions, and is being phased out.

Proposed Solution

The affected tests should be ported to use tpu-client-next. Consider using ClientBuilder to streamline the process.

There are many tests to cover, so consider filing PRs to migrate tests to tpu-client-next, no more than 3 tests per PR so we can keep review cycles short. Make sure you indicate on this issue which tests are you working on before making code changes to avoid conflicts across contributors.

[Vinayapr23]

Can i work on this?

[alexpyattaev]

Can i work on this?

Of course!

[Vinayapr23]

Can i work on this?

Of course!

I think ill work on
test_replay_stage_refresh_last_vote() inside ./core/src/replay_stage.rs
test_tvu_exit() inside ./core/src/tvu.rs

[onenewcode]

I want to work on test_replay_stage_last_vote_outside_slot_hashes() inside ./core/src/replay_stage.rs

[onenewcode]

I have a question. Currently in replay_stage.rs, all ConnectionCache::new_quic_for_tests are bound to crate::voting_service::VotingService::handle_vote, but handle_vote uses ConnectionCache. So what we need to do is generate a new ConnectionCache using tpu-client-next? Since you mentioned that ConnectionCache will be phased out, I'm unsure whether to proceed with this approach?

Here is the relevant part of the code:

// /core/src/replay_stage.rs
      let connection_cache = if DEFAULT_VOTE_USE_QUIC {
            ConnectionCache::new_quic_for_tests(
                "connection_cache_vote_quic",
                DEFAULT_TPU_CONNECTION_POOL_SIZE,
            )
        } else {
            ConnectionCache::with_udp(
                "connection_cache_vote_udp",
                DEFAULT_TPU_CONNECTION_POOL_SIZE,
            )
        };

        crate::voting_service::VotingService::handle_vote(
            &cluster_info,
            &poh_recorder,
            &tower_storage,
            vote_info,
            Arc::new(connection_cache),
        );

// /core/src/voting_service.rs
pub struct VotingService {
    thread_hdl: JoinHandle<()>,
}

impl VotingService {

    pub fn handle_vote(
        cluster_info: &ClusterInfo,
        poh_recorder: &RwLock<PohRecorder>,
        tower_storage: &dyn TowerStorage,
        vote_op: VoteOp,
        connection_cache: Arc<ConnectionCache>,
    ) {
    }

}

[illegalcall]

Hi @alexpyattaev, I also wanted to work on this, but looks like the three known tests are already claimed.
If i am mistaken please point me to those, otherwise if @Vinayapr23 or @onenewcode need any help with these, please reach out. Happy to share tasks and contribute.

[illegalcall]

Hey @alexpyattaev , I've opened a PR to migrate the VotingService-related tests in replay_stage.rs to use tpu-client-next. This covers 4 tests (slightly over the 3-test guideline) since they share identical setup and benefit from a common VoteTransportClient abstraction.

Will open follow-up PRs for remaining usages once this is reviewed.

[Vinayapr23]

Hey @alexpyattaev , I've opened a PR to migrate the VotingService-related tests in replay_stage.rs to use tpu-client-next. This covers 4 tests (slightly over the 3-test guideline) since they share identical setup and benefit from a common VoteTransportClient abstraction.

Will open follow-up PRs for remaining usages once this is reviewed.

Hey @illegalcall
I was working on test_replay_stage_refresh_last_vote() in replay_stage.rs
i believe you modified that test also as I can see from your PR, apart from the 4 tests you already modified
I request you to retrace so I can continue there
Otherwise this will cause conflicts @alexpyattaev

[alexpyattaev]

please keep PRs small, no awards for modifying more tests at once, only more conflicts.

[nullopcode0]

Hey — I see the test usages in core/ are mostly claimed. There are 3 remaining usages in the cli/ crate that haven't been addressed:

cli/src/cli.rs:964 — inside CliCommand::Ping handler
cli/src/program.rs:3311 — inside send_deploy_messages()
cli/src/program_v4.rs:1331 — inside send_messages()
These are production code paths using the #[cfg(feature = "dev-context-only-utils")] pattern rather than test functions. Are these in-scope for this issue? If so, happy to take them on in a single PR (all 3 follow the same pattern).

[alexpyattaev]

Anything that uses ConnectionCache is eligible. But please let us make small, focused PRs, even if they are repetative. It ends up working faster and produces less bugs.

[nullopcode0]

Got it — small focused PRs, one at a time. I'll start with the first ConnectionCache::new_quic_for_tests usage in the CLI code and open a PR for that. Appreciate the guidance.