Skip to content

Commit 1605886

Browse files
glozowglozow
committed
[validation] return effective feerate from mempool validation
1 parent 5d35b4a commit 1605886

File tree

3 files changed

+52
-8
lines changed

3 files changed

+52
-8
lines changed

‎src/test/txpackage_tests.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,14 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
9595
BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end());
9696
BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(),
9797
"Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason());
98+
BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == COIN);
9899
BOOST_CHECK(it_child != result_parent_child.m_tx_results.end());
99100
BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(),
100101
"Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason());
101102
BOOST_CHECK(result_parent_child.m_package_feerate.has_value());
102103
BOOST_CHECK(result_parent_child.m_package_feerate.value() ==
103104
CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)));
105+
BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN);
104106

105107
// A single, giant transaction submitted through ProcessNewPackage fails on single tx policy.
106108
CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
@@ -323,8 +325,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
323325
auto it_child = submit_parent_child.m_tx_results.find(tx_child->GetWitnessHash());
324326
BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end());
325327
BOOST_CHECK(it_parent->second.m_state.IsValid());
328+
BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_parent)));
326329
BOOST_CHECK(it_child != submit_parent_child.m_tx_results.end());
327330
BOOST_CHECK(it_child->second.m_state.IsValid());
331+
BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_child)));
328332

329333
BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
330334
BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash())));
@@ -613,6 +617,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
613617
BOOST_CHECK_MESSAGE(mixed_result.m_package_feerate.value() == expected_feerate,
614618
strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
615619
mixed_result.m_package_feerate.value().ToString()));
620+
BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate);
621+
BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate);
616622
}
617623
}
618624

@@ -695,6 +701,8 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
695701

696702
const CFeeRate expected_feerate(coinbase_value - child_value,
697703
GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child));
704+
BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate);
705+
BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate);
698706
BOOST_CHECK(expected_feerate.GetFeePerK() > 1000);
699707
BOOST_CHECK(submit_cpfp.m_package_feerate.has_value());
700708
BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate,
@@ -756,6 +764,16 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
756764
BOOST_CHECK_MESSAGE(submit_prioritised_package.m_package_feerate.value() == expected_feerate,
757765
strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
758766
submit_prioritised_package.m_package_feerate.value().ToString()));
767+
auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash());
768+
auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash());
769+
BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end());
770+
BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID);
771+
BOOST_CHECK(it_parent->second.m_base_fees.value() == 0);
772+
BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate);
773+
BOOST_CHECK(it_child != submit_prioritised_package.m_tx_results.end());
774+
BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID);
775+
BOOST_CHECK(it_child->second.m_base_fees.value() == 200);
776+
BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate);
759777
}
760778

761779
// Package feerate is calculated without topology in mind; it's just aggregating fees and sizes.
@@ -801,6 +819,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
801819
BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == "");
802820
BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee,
803821
strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value()));
822+
BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich)));
804823

805824
BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
806825
BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash())));

‎src/validation.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,11 @@ class MemPoolAccept
583583
/** Total virtual size of all transactions being replaced. */
584584
size_t m_conflicting_size{0};
585585

586+
/** If we're doing package validation (i.e. m_package_feerates=true), the "effective"
587+
* package feerate of this transaction is the total fees divided by the total size of
588+
* transactions (which may include its ancestors and/or descendants). */
589+
CFeeRate m_package_feerate{0};
590+
586591
const CTransactionRef& m_ptx;
587592
/** Txid. */
588593
const uint256& m_hash;
@@ -1147,9 +1152,11 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
11471152
// Find the wtxids of the transactions that made it into the mempool. Allow partial submission,
11481153
// but don't report success unless they all made it into the mempool.
11491154
for (Workspace& ws : workspaces) {
1155+
const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate :
1156+
CFeeRate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
11501157
if (m_pool.exists(GenTxid::Wtxid(ws.m_ptx->GetWitnessHash()))) {
11511158
results.emplace(ws.m_ptx->GetWitnessHash(),
1152-
MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees));
1159+
MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate));
11531160
GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence());
11541161
} else {
11551162
all_submitted = false;
@@ -1177,16 +1184,17 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
11771184

11781185
if (!ConsensusScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state);
11791186

1187+
const CFeeRate effective_feerate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
11801188
// Tx was accepted, but not added
11811189
if (args.m_test_accept) {
1182-
return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees);
1190+
return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate);
11831191
}
11841192

11851193
if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state);
11861194

11871195
GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence());
11881196

1189-
return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees);
1197+
return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate);
11901198
}
11911199

11921200
PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args)
@@ -1245,18 +1253,21 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
12451253
}
12461254

12471255
for (Workspace& ws : workspaces) {
1256+
ws.m_package_feerate = package_feerate;
12481257
if (!PolicyScriptChecks(args, ws)) {
12491258
// Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished.
12501259
package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
12511260
results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
12521261
return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
12531262
}
12541263
if (args.m_test_accept) {
1264+
const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate :
1265+
CFeeRate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
12551266
// When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are
12561267
// no further mempool checks (passing PolicyScriptChecks implies passing ConsensusScriptChecks).
12571268
results.emplace(ws.m_ptx->GetWitnessHash(),
12581269
MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions),
1259-
ws.m_vsize, ws.m_base_fees));
1270+
ws.m_vsize, ws.m_base_fees, effective_feerate));
12601271
}
12611272
}
12621273

‎src/validation.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ struct MempoolAcceptResult {
134134
const std::optional<int64_t> m_vsize;
135135
/** Raw base fees in satoshis. */
136136
const std::optional<CAmount> m_base_fees;
137+
/** The feerate at which this transaction was considered. This includes any fee delta added
138+
* using prioritisetransaction (i.e. modified fees). If this transaction was submitted as a
139+
* package, this is the package feerate, which may also include its descendants and/or
140+
* ancestors. Only present when m_result_type = ResultType::VALID. */
141+
const std::optional<CFeeRate> m_effective_feerate;
137142

138143
// The following field is only present when m_result_type = ResultType::DIFFERENT_WITNESS
139144
/** The wtxid of the transaction in the mempool which has the same txid but different witness. */
@@ -143,8 +148,11 @@ struct MempoolAcceptResult {
143148
return MempoolAcceptResult(state);
144149
}
145150

146-
static MempoolAcceptResult Success(std::list<CTransactionRef>&& replaced_txns, int64_t vsize, CAmount fees) {
147-
return MempoolAcceptResult(std::move(replaced_txns), vsize, fees);
151+
static MempoolAcceptResult Success(std::list<CTransactionRef>&& replaced_txns,
152+
int64_t vsize,
153+
CAmount fees,
154+
CFeeRate effective_feerate) {
155+
return MempoolAcceptResult(std::move(replaced_txns), vsize, fees, effective_feerate);
148156
}
149157

150158
static MempoolAcceptResult MempoolTx(int64_t vsize, CAmount fees) {
@@ -164,9 +172,15 @@ struct MempoolAcceptResult {
164172
}
165173

166174
/** Constructor for success case */
167-
explicit MempoolAcceptResult(std::list<CTransactionRef>&& replaced_txns, int64_t vsize, CAmount fees)
175+
explicit MempoolAcceptResult(std::list<CTransactionRef>&& replaced_txns,
176+
int64_t vsize,
177+
CAmount fees,
178+
CFeeRate effective_feerate)
168179
: m_result_type(ResultType::VALID),
169-
m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees) {}
180+
m_replaced_transactions(std::move(replaced_txns)),
181+
m_vsize{vsize},
182+
m_base_fees(fees),
183+
m_effective_feerate(effective_feerate) {}
170184

171185
/** Constructor for already-in-mempool case. It wouldn't replace any transactions. */
172186
explicit MempoolAcceptResult(int64_t vsize, CAmount fees)

0 commit comments

Comments
 (0)