Improve TestPrepareProposalConsistency

[evan-forbes]

The TestPrepareProposalConsistency test uses randomly sized transactions to test if PrepareProposal will ever create a proposal block that ProcessProposal rejects. While other tests are responsible for ensuring that a transaction that gets through CheckTx, gets through PrepareProposal, and then ProcessProposal, this test can still be improved upon by being more exact with how many transactions are expected to be included in the block.

celestia-app/app/test/fuzz_abci_test.go

Line 144 in b59e876

require.GreaterOrEqual(t, len(resp.BlockData.Txs), sendTxCount+1)

[ThanhNhann]

Hey @evan-forbes, can I handle this issue?

[ThanhNhann]

Hi @evan-forbes, I see the code from branch main now have some differs from your code reference above, do we still need to improve it now?

[rootulp]

Thanks for your interest @ThanhNhann ! It looks like this assertion still exists on main

celestia-app/app/test/fuzz_abci_test.go

Line 154 in c5e1786

require.GreaterOrEqual(t, len(resp.BlockData.Txs), sendTxCount+1)

and I think this issue wants a more exact assertion for the # of transactions (blob and non-blob) that get included in the process proposal block. Perhaps it can assert that the # of transactions in the prepare proposal response == the # of transactions in the process proposal response (?)

[exp7l]

Greetings! @evan-forbes @rootulp
Looks like this is still open. Can I work on it?

[exp7l]

Looking

[rootulp]

Closing this as won't do because it's not immediately clear what this issue is asking for. See #3087 (comment)

cc: @evan-forbes please re-open with acceptance criteria if we still want this.

[evan-forbes]

this test can still be improved upon by being more exact with how many transactions are expected to be included in the block.

I think this is the core piece, we aren't actually checking how many transactions are being included, only that one blobTx did. This means that the rest of the blob txs could be filtered out. Ideally we would make this more restrictive. This might involve writing a different or doing some analysis for what conservative % of txs should be expected to get through.

celestia-app/app/test/fuzz_abci_test.go

Lines 149 to 155 in 949a2d5

	require.Equal(t, abci.ResponseProcessProposal_ACCEPT, res.Result)
	// At least all of the send transactions and one blob tx
	// should make it into the block. This should be expected to
	// change if PFB transactions are not separated and put into
	// their own namespace
	require.GreaterOrEqual(t, len(resp.BlockData.Txs), sendTxCount+1)
	}