program-test: Expose bank task to fix fuzzing

[joncinque]

Problem

Program-test creates a running task to register ticks on the bank, but does not expose this task. For most cases, this works great, but in a fuzzing environment, not closing the thread properly blows up /tmp with all sorts of directories like /tmp/.tmp0000, perhaps related to rocksdb? I'm not too familiar with that side of it.

Summary of Changes

Expose the running task on the bank through a new return type, ProgramTestState, and allow it to be stopped on drop. This changes the API on ProgramTest.start(), which seems reasonable considering most of these are tests users. If needed, I can spin off a new / old function and mark this as deprecated.

One thing to note is that we can't simply destructure this as:

let ProgramTestState { mut banks_client, payer, last_blockhash, .. } = program_test.start().await;

Because the task gets dropped at that point.

The other nice part of this approach is that we can start to support a lot of functions that have been asked for by devs. My next idea is to add a warp_to_slot on ProgramTestState. We'll just need to keep track of the BankForks and use Bank::warp_from_parent.

This approach gets much more consistent performance from fuzzing as well. For the test, we can't easily use the fuzz! macro because then we need to build with cargo hfuzz build, so instead it's simulating what a fuzz run would look like with arbitrary &[u8] data.

Fixes #14707

[mvines]

I like the direction. We needed a place to hook up a clock/slot manipulation interface and this provides such a location

[mvines]

I think we can limit scope here

Suggested change

	pub fn new(
	fn new(

[joncinque]

Good call, I was thinking someone might want to hack on it from the outside, but this'll be safer

[mvines]

nit: I think I fancy "context" more:

Suggested change

	pub struct ProgramTestState {
	pub struct ProgramTestContext {

[joncinque]

Works for me!

[mvines]

I'm definitely concerned about changing this signature and blowing up existing users. I think we need to keep it for existing users and future users that just want to keep it simple.

Some ideas for a new method name for this signature:

1. enter() -- you enter the ProgramTextContext
2. run() -- confusing, do I "start" or "run"
3. start_with_context() -- maybe my favourite of the three, consistent with start() and the Rust norms of adding "with" methods that enhance the functionality of the base method

[joncinque]

Ok no problem, I'll add start_with_context()

[mvines]

nice work

[mvines]

Suggested change

	let (tx, mut rx) = mpsc::channel(1);
	let (sender, mut receiver) = mpsc::channel(1);

🙏🏼. Most people unfortunately like to use tx to mean transaction in the code base, except when it means something else like in this case

[joncinque]

Ah, right, on it switching to AtomicBool

[mvines]

An AtomicBool is generally used in other parts of the code for this kind of exit pattern. I'm not opposed to a channel if you want to keep it though.

Here's one example of using the atomic bool:

solana/core/src/transaction_status_service.rs

Line 33 in d6873b8

if exit.load(Ordering::Relaxed) {

[joncinque]

That was actually my initial approach, but then I thought a message would be clearer. I'll change it to an AtomicBool to keep things consistent in the codebase

[codecov]

Codecov Report

Merging #14908 (a100a36) into master (b948ede) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #14908     +/-   ##
=========================================
- Coverage    79.9%    79.8%   -0.1%     
=========================================
  Files         401      401             
  Lines      100698   100781     +83     
=========================================
- Hits        80531    80503     -28     
- Misses      20167    20278    +111     

Pull request has been modified.