-
Notifications
You must be signed in to change notification settings - Fork 11
test: add fee timer blocking test coverage #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,80 +44,18 @@ BOOST_AUTO_TEST_CASE(client_tests) | |
|
|
||
| tester.handshake(); | ||
|
|
||
| // After the handshake the client must send a SetupConnection message to the | ||
| // Template Provider. | ||
|
|
||
| tester.handshake(); | ||
| BOOST_TEST_MESSAGE("Handshake done, send SetupConnectionMsg"); | ||
|
|
||
| node::Sv2NetMsg setup{tester.SetupConnectionMsg()}; | ||
| tester.receiveMessage(setup); | ||
| // SetupConnection.Success is 6 bytes | ||
| BOOST_REQUIRE_EQUAL(tester.PeerReceiveBytes(), SV2_HEADER_ENCRYPTED_SIZE + 6 + Poly1305::TAGLEN); | ||
| tester.SendSetupConnection(); | ||
|
|
||
| // There should be no block templates before any client gave us their coinbase | ||
| // output data size: | ||
| BOOST_REQUIRE(tester.GetBlockTemplateCount() == 0); | ||
|
|
||
| std::vector<uint8_t> coinbase_output_constraint_bytes{ | ||
| 0x01, 0x00, 0x00, 0x00, // coinbase_output_max_additional_size | ||
| 0x00, 0x00 // coinbase_output_max_sigops | ||
| }; | ||
| node::Sv2NetMsg msg{node::Sv2MsgType::COINBASE_OUTPUT_CONSTRAINTS, std::move(coinbase_output_constraint_bytes)}; | ||
| tester.receiveMessage(msg); | ||
| tester.SendCoinbaseOutputConstraints(); | ||
| BOOST_TEST_MESSAGE("The reply should be NewTemplate and SetNewPrevHash"); | ||
| // Payload sizes for fixed-layout SV2 messages used in this test | ||
| constexpr size_t SV2_SET_NEW_PREV_HASH_MESSAGE_SIZE = 8 + 32 + 4 + 4 + 32; // = 80 | ||
| constexpr size_t SV2_NEW_TEMPLATE_MESSAGE_SIZE = | ||
| 8 + // template_id | ||
| 1 + // future_template | ||
| 4 + // version | ||
| 4 + // coinbase_tx_version | ||
| 2 + // coinbase_prefix (CompactSize(1) + 1-byte OP_0) | ||
| 4 + // coinbase_tx_input_sequence | ||
| 8 + // coinbase_tx_value_remaining | ||
| 4 + // coinbase_tx_outputs_count (2 - mock creates 3, only 2 OP_RETURN outputs pass filter) | ||
| 2 + 56 + // B0_64K: length prefix (2 bytes) + 2 outputs (witness commitment 43 bytes + merge mining 13 bytes) | ||
| 4 + // coinbase_tx_locktime | ||
| 1; // merkle_path count (CompactSize(0)) | ||
|
|
||
| // Two messages (SetNewPrevHash + NewTemplate) may arrive in one read or sequentially. | ||
| const size_t expected_set_new_prev_hash = SV2_HEADER_ENCRYPTED_SIZE + SV2_SET_NEW_PREV_HASH_MESSAGE_SIZE + Poly1305::TAGLEN; | ||
| const size_t expected_new_template = SV2_HEADER_ENCRYPTED_SIZE + SV2_NEW_TEMPLATE_MESSAGE_SIZE + Poly1305::TAGLEN; | ||
| const size_t expected_pair_bytes = expected_set_new_prev_hash + expected_new_template; | ||
|
|
||
| const auto expect_template_pair = [&](const char* context) { | ||
| size_t accumulated = 0; | ||
| bool seen_prev_hash = false; | ||
| bool seen_new_template = false; | ||
| int iterations = 0; | ||
|
|
||
| while (accumulated < expected_pair_bytes) { | ||
| size_t chunk = tester.PeerReceiveBytes(); | ||
| accumulated += chunk; | ||
| ++iterations; | ||
|
|
||
| if (chunk == expected_set_new_prev_hash) { | ||
| seen_prev_hash = true; | ||
| } else if (chunk == expected_new_template) { | ||
| seen_new_template = true; | ||
| } else if (chunk == expected_pair_bytes) { | ||
| seen_prev_hash = true; | ||
| seen_new_template = true; | ||
| break; | ||
| } else { | ||
| BOOST_FAIL(std::string("Unexpected message size while receiving ") + context); | ||
| } | ||
|
|
||
| BOOST_REQUIRE_MESSAGE(iterations <= 2, std::string("Too many fragments for ") + context); | ||
| } | ||
| tester.ReceiveTemplatePair(); | ||
|
|
||
| BOOST_REQUIRE_MESSAGE(seen_prev_hash, std::string("Missing SetNewPrevHash during ") + context); | ||
| BOOST_REQUIRE_MESSAGE(seen_new_template, std::string("Missing NewTemplate during ") + context); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you've lost part of this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've restored the lost bit
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They still seem missing as of 05585cd.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a good catch, let me reset and redo. Thank you |
||
| BOOST_REQUIRE_MESSAGE(accumulated == expected_pair_bytes, std::string("Incomplete response for ") + context); | ||
| }; | ||
|
|
||
| expect_template_pair("initial template broadcast"); | ||
| const size_t expected_new_template = SV2_HEADER_ENCRYPTED_SIZE + TPTester::SV2_NEW_TEMPLATE_MSG_SIZE + Poly1305::TAGLEN; | ||
|
|
||
| // There should now be one template | ||
| BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 1); | ||
|
|
@@ -144,7 +82,7 @@ BOOST_AUTO_TEST_CASE(client_tests) | |
| BOOST_TEST_MESSAGE("Receive NewTemplate (fee increase)"); | ||
| // One NewTemplate follows: header + payload + payload tag | ||
| size_t bytes_fee_nt = tester.PeerReceiveBytes(); | ||
| BOOST_REQUIRE_EQUAL(bytes_fee_nt, SV2_HEADER_ENCRYPTED_SIZE + SV2_NEW_TEMPLATE_MESSAGE_SIZE + Poly1305::TAGLEN); | ||
| BOOST_REQUIRE_EQUAL(bytes_fee_nt, expected_new_template); | ||
|
|
||
| // Get the latest template id | ||
| uint64_t template_id = 0; | ||
|
|
@@ -203,7 +141,7 @@ BOOST_AUTO_TEST_CASE(client_tests) | |
|
|
||
| // Expect our peer to receive a NewTemplate message | ||
| size_t bytes_second_nt = tester.PeerReceiveBytes(); | ||
| BOOST_REQUIRE_EQUAL(bytes_second_nt, SV2_HEADER_ENCRYPTED_SIZE + SV2_NEW_TEMPLATE_MESSAGE_SIZE + Poly1305::TAGLEN); | ||
| BOOST_REQUIRE_EQUAL(bytes_second_nt, expected_new_template); | ||
|
|
||
| // Check that there's a new template | ||
| BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 3); | ||
|
|
@@ -234,7 +172,7 @@ BOOST_AUTO_TEST_CASE(client_tests) | |
| BOOST_REQUIRE(tester.m_mining_control->WaitForTemplateSeq(seq_after_second_nt + 1)); | ||
|
|
||
| // We should send out another NewTemplate and SetNewPrevHash (two messages) | ||
| expect_template_pair("new tip template broadcast"); | ||
| tester.ReceiveTemplatePair(); | ||
| // The SetNewPrevHash message is redundant | ||
| // TODO: don't send it? | ||
| // Background: in the future we want to send an empty or optimistic template | ||
|
|
@@ -268,4 +206,49 @@ BOOST_AUTO_TEST_CASE(client_tests) | |
| tester.m_mining_control->Shutdown(); | ||
| } | ||
|
|
||
| // Test fee-based rate limiting timer (-sv2interval flag). | ||
| // Uses is_test=false to exercise actual timer logic. | ||
| BOOST_AUTO_TEST_CASE(fee_timer_blocking_test) | ||
| { | ||
| // Use real wall-clock time instead of mock time | ||
| SetMockTime(std::chrono::seconds{0}); | ||
|
|
||
| Sv2TemplateProviderOptions opts; | ||
| opts.is_test = false; | ||
| opts.fee_check_interval = std::chrono::seconds{2}; | ||
| TPTester tester{opts}; | ||
|
|
||
| tester.handshake(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why the original has two handshake calls, that might be a bug. It would be good to reduce the amount of duplicated test code, that also makes it more clear how each test case is different.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let me do a clean up and have a much clearer approach
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. d54d4b7 is better, but the code between
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I have addressed it in ef1a258 . Thank you. |
||
| tester.SendSetupConnection(); | ||
| tester.SendCoinbaseOutputConstraints(); | ||
| tester.ReceiveTemplatePair(); | ||
|
|
||
| const size_t expected_new_template = SV2_HEADER_ENCRYPTED_SIZE + TPTester::SV2_NEW_TEMPLATE_MSG_SIZE + Poly1305::TAGLEN; | ||
|
|
||
| uint64_t seq = tester.m_mining_control->GetTemplateSeq(); | ||
|
|
||
| // Trigger a fee increase immediately after template; timer should block it | ||
| BOOST_TEST_MESSAGE("Trigger fee increase while timer is blocking"); | ||
| std::vector<CTransactionRef> blocked_fee_txs{MakeDummyTx()}; | ||
| tester.m_mining_control->TriggerFeeIncrease(blocked_fee_txs); | ||
|
|
||
| bool got_template = tester.m_mining_control->WaitForTemplateSeq(seq + 1, std::chrono::milliseconds{2500}); | ||
| BOOST_REQUIRE_MESSAGE(!got_template, "Fee increase should be blocked when timer hasn't fired"); | ||
| BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 1); | ||
|
|
||
| // After fee_check_interval (2s), the timer should allow fee checks | ||
| BOOST_TEST_MESSAGE("Trigger fee increase after timer fires"); | ||
| std::vector<CTransactionRef> allowed_fee_txs{MakeDummyTx()}; | ||
| tester.m_mining_control->TriggerFeeIncrease(allowed_fee_txs); | ||
|
|
||
| got_template = tester.m_mining_control->WaitForTemplateSeq(seq + 1, std::chrono::milliseconds{3000}); | ||
| BOOST_REQUIRE_MESSAGE(got_template, "Fee increase should be allowed after timer fires"); | ||
|
|
||
| size_t bytes_nt = tester.PeerReceiveBytes(); | ||
| BOOST_REQUIRE_EQUAL(bytes_nt, expected_new_template); | ||
| BOOST_REQUIRE_EQUAL(tester.GetBlockTemplateCount(), 2); | ||
|
|
||
| tester.m_mining_control->Shutdown(); | ||
| } | ||
|
|
||
| BOOST_AUTO_TEST_SUITE_END() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
05585cd: nit, this in the wrong commit now (don't worry about it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me do a better cleanup, thank you.