Skip to content

Commit 59e6f71

Browse files
evoskuilevoskuil
committed
Fix/update tx confirm & metadata for use in network.
1 parent b0b85fa commit 59e6f71

10 files changed

Lines changed: 57 additions & 107 deletions

File tree

‎include/bitcoin/system/chain/block.hpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,6 @@ class BC_API block
198198
bool is_signature_operations_limited(bool bip16,
199199
bool bip141) const NOEXCEPT;
200200

201-
/// Requires input.metadata.spent (prevout confirmation).
202-
bool is_unspent_coinbase_collision() const NOEXCEPT;
203-
204201
private:
205202
typedef struct { size_t nominal; size_t witnessed; } sizes;
206203

‎include/bitcoin/system/chain/input.hpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class BC_API input
113113
/// Assumes coinbase if prevout not populated (returns only legacy sigops).
114114
size_t signature_operations(bool bip16, bool bip141) const NOEXCEPT;
115115

116-
/// Requires metadata.height and median_time_past (otherwise returns true).
116+
/// Requires metadata.prevout_height and median_time_past (otherwise true).
117117
bool is_relative_locked(size_t height,
118118
uint32_t median_time_past) const NOEXCEPT;
119119

@@ -156,9 +156,8 @@ class BC_API input
156156

157157
public:
158158
/// Public mutable metadata access, copied but not compared for equality.
159-
/// Defaults are set so non-population issues usually imply invalidity.
160159
mutable chain::output::cptr prevout{};
161-
mutable chain::prevout metadata{ zero, max_uint32, true, true, true };
160+
mutable chain::prevout metadata{};
162161
};
163162

164163
typedef std_vector<input> inputs;

‎include/bitcoin/system/chain/prevout.hpp‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,13 @@ class BC_API prevout final
3535
///************************************************************************
3636
union
3737
{
38-
/// The confirmed chain height of the prevout (zero if not found).
39-
/// Unused if the input owning this prevout is null (coinbase).
40-
size_t height;
38+
/// Unused for coinbase.
39+
/// The confirmed height of the prevout or max_uint32.
40+
size_t prevout_height{ max_uint32 };
4141

42-
/// node: populated with a database identifier for the parent tx.
43-
uint32_t parent{ zero };
42+
/// node: populated with database identifier for the parent tx.
43+
/// Link::terminal must derive from default of max_uint32.
44+
uint32_t parent_tx;
4445
};
4546

4647
///************************************************************************
@@ -49,35 +50,27 @@ class BC_API prevout final
4950
///************************************************************************
5051
union
5152
{
52-
/// The median time past at height (max_uint32 if not found/confirmed).
53-
/// Unused if the input owning this prevout is null (coinbase).
53+
/// Unused for coinbase.
54+
/// The median time past at confirmed prevout block or max_uint32.
5455
uint32_t median_time_past{ max_uint32 };
5556

56-
/// node: set to the database record of the input association.
57-
/// node: it is necessary that Link::terminal derives from max_uint32.
58-
uint32_t link;
57+
/// node: populated with database identifier for the input/point.
58+
/// Link::terminal must derive from default of max_uint32.
59+
uint32_t point_link;
5960
};
6061

6162
///************************************************************************
6263
/// CONSENSUS:
6364
/// An unspent coinbase collision is immature (unspendable) and spent
6465
/// collision is mature [bip30]. CB collision presumed precluded by bip34.
66+
/// This is NOT guarded by system::chain confirmation checks.
6567
///************************************************************************
66-
union
67-
{
68-
/// If input owning this prevout is null (coinbase), this implies that
69-
/// all outputs of any duplicate txs are fully spent at height.
70-
/// If the input owning this prevout is not null (not coinbase), this
71-
/// indicates whether the prevout is spent at height (double spend).
72-
bool spent{ true };
73-
74-
/// node: indicates that the input spends output inside same block.
75-
bool inside;
76-
};
68+
/// The confirmed height of the spender or max_uint32.
69+
uint32_t spender_height{ max_uint32 };
7770

78-
/// node: set via block.populate() as internal spends do not
79-
/// require prevout block association for relative locktime checks.
80-
/// So median_time_past is not required as locked is determined here.
71+
/// node: set via block.populate() as internal spends do not require
72+
/// prevout block association for relative locktime checks. So
73+
/// median_time_past is not required as locked is determined here.
8174
bool locked{ true };
8275

8376
/// The previous output is of a coinbase transaction.

‎include/bitcoin/system/chain/transaction.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,17 @@ class BC_API transaction
224224
/// Confirm (requires confirmation order).
225225
/// -----------------------------------------------------------------------
226226

227-
/// Requires input.metadata.height/median_time_past (prevout confirmation).
227+
/// Requires input.metadata.prevout_height/median_time_past.
228228
bool is_relative_locked(size_t height,
229229
uint32_t median_time_past) const NOEXCEPT;
230230

231-
/// Requires input.metadata.height (prevout confirmation).
231+
/// Requires input.metadata.prevout_height.
232232
bool is_immature(size_t height) const NOEXCEPT;
233233

234-
/// Requires input.metadata.height (prevout confirmation).
234+
/// Requires input.metadata.prevout_height.
235235
bool is_unconfirmed_spend(size_t height) const NOEXCEPT;
236236

237-
/// Requires input.metadata.height/spent (prevout confirmation).
237+
/// Requires input.metadata.spender_height.
238238
bool is_confirmed_double_spend(size_t height) const NOEXCEPT;
239239

240240
private:

‎src/chain/block.cpp‎

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -750,20 +750,6 @@ bool block::is_signature_operations_limited(bool bip16,
750750
return signature_operations(bip16, bip141) > limit;
751751
}
752752

753-
//*****************************************************************************
754-
// CONSENSUS: check excluded under bip30 exception blocks and bip30_deactivate
755-
// until bip30_reactivate. Those conditions are rolled up into the bip30 flag.
756-
//*****************************************************************************
757-
bool block::is_unspent_coinbase_collision() const NOEXCEPT
758-
{
759-
if (txs_->empty() || txs_->front()->inputs_ptr()->empty())
760-
return false;
761-
762-
// May only commit duplicate coinbase that is already confirmed spent.
763-
// Metadata population defaults coinbase to spent (not a collision).
764-
return !txs_->front()->inputs_ptr()->front()->metadata.spent;
765-
}
766-
767753
// Search is unordered, forward refs (and duplicates) caught by block.check.
768754
void block::populate() const NOEXCEPT
769755
{
@@ -798,6 +784,8 @@ code block::populate_with_metadata(const chain::context& ctx) const NOEXCEPT
798784
if (txs_->empty())
799785
return error::block_success;
800786

787+
const auto& self = txs_->front()->get_hash(false);
788+
constexpr auto matures = !is_zero(coinbase_maturity);
801789
const auto bip68 = ctx.is_enabled(chain::flags::bip68_rule);
802790
unordered_map_of_cref_point_to_output_cptr_cref points(outputs());
803791
uint32_t index{};
@@ -807,27 +795,24 @@ code block::populate_with_metadata(const chain::context& ctx) const NOEXCEPT
807795
for (const auto& out: *(*tx)->outputs_ptr())
808796
points.emplace(cref_point{ (*tx)->get_hash(false), index++ }, out);
809797

810-
// Populate input prevouts from hash table and obtain maturity.
798+
// Populate prevouts from hash table, determine get locked and maturity.
811799
for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
812800
{
813801
for (const auto& in: *(*tx)->inputs_ptr())
814802
{
815803
// Map chain::point to cref_point for search, should optimize away.
816-
const auto point = points.find({ in->point().hash(),
817-
in->point().index() });
804+
const cref_point key{ in->point().hash(), in->point().index() };
818805

819-
if (point != points.end())
806+
if (const auto it = points.find(key); it != points.end())
820807
{
821-
// Zero maturity coinbase spend is immature.
808+
const auto immature = (matures && in->point().hash() == self);
822809
const auto lock = (bip68 && (*tx)->is_internally_locked(*in));
823-
const auto immature = !is_zero(coinbase_maturity) &&
824-
(in->point().hash() == txs_->front()->get_hash(false));
810+
in->prevout = it->second;
825811

826-
in->prevout = point->second;
812+
// If invalid shortcircuit population and return above error.
827813
if ((in->metadata.locked = (immature || lock)))
828814
{
829-
// Shortcircuit population and return above error.
830-
return immature ? error::coinbase_maturity :
815+
return immature ? error::coinbase_maturity :
831816
error::relative_time_locked;
832817
}
833818
}
@@ -996,11 +981,6 @@ code block::accept(const context& ctx, size_t subsidy_interval,
996981
// This assumes that prevout and metadata caching are completed on all inputs.
997982
code block::confirm(const context& ctx) const NOEXCEPT
998983
{
999-
const auto bip30 = ctx.is_enabled(bip30_rule);
1000-
1001-
if (bip30 && is_unspent_coinbase_collision())
1002-
return error::unspent_coinbase_collision;
1003-
1004984
return confirm_transactions(ctx);
1005985
}
1006986

‎src/chain/input.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,8 @@ bool input::is_relative_locked(size_t height,
427427
uint32_t median_time_past) const NOEXCEPT
428428
{
429429
// Prevout must be found and height/median_time_past metadata populated.
430-
////BC_ASSERT(!is_zero(metadata.height));
431430
return is_relative_locked(sequence_, height, median_time_past,
432-
metadata.height, metadata.median_time_past);
431+
metadata.prevout_height, metadata.median_time_past);
433432
}
434433

435434
bool input::reserved_hash(hash_cref& out) const NOEXCEPT

‎src/chain/transaction.cpp‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -716,8 +716,8 @@ bool transaction::is_immature(size_t height) const NOEXCEPT
716716
const auto mature = [=](const auto& input) NOEXCEPT
717717
{
718718
return input->metadata.coinbase ?
719-
is_coinbase_mature(input->metadata.height, height) :
720-
is_non_coinbase_mature(input->metadata.height, height);
719+
is_coinbase_mature(input->metadata.prevout_height, height) :
720+
is_non_coinbase_mature(input->metadata.prevout_height, height);
721721
};
722722

723723
return !std::all_of(inputs_->begin(), inputs_->end(), mature);
@@ -744,7 +744,7 @@ bool transaction::is_internally_locked(const input& in) const NOEXCEPT
744744
return false;
745745

746746
// Internal spends have no relative height/mtp (own metadata vs. itself).
747-
return in.is_relative_locked(in.metadata.height,
747+
return in.is_relative_locked(in.metadata.prevout_height,
748748
in.metadata.median_time_past);
749749
}
750750

@@ -767,18 +767,15 @@ bool transaction::is_relative_locked(size_t height,
767767
return std::any_of(inputs_->begin(), inputs_->end(), locked);
768768
}
769769

770-
// Spends internal to a block are handled by block validation.
771770
bool transaction::is_unconfirmed_spend(size_t height) const NOEXCEPT
772771
{
773772
BC_ASSERT(!is_coinbase());
774773

775-
// Zero is either genesis or not found.
776-
// Test maturity first to obtain proper error code.
777-
// Spends internal to a block are handled by block validation.
778774
const auto unconfirmed = [=](const auto& input) NOEXCEPT
779775
{
780-
const auto prevout_height = input->metadata.height;
781-
return is_zero(prevout_height) && !(height > prevout_height);
776+
// Spends internal to a block are handled by block validation.
777+
// The lack of equality check here prevents conflict with self.
778+
return height < input->metadata.prevout_height;
782779
};
783780

784781
return std::any_of(inputs_->begin(), inputs_->end(), unconfirmed);
@@ -788,10 +785,11 @@ bool transaction::is_confirmed_double_spend(size_t height) const NOEXCEPT
788785
{
789786
BC_ASSERT(!is_coinbase());
790787

791-
// Spends internal to a block are handled by block validation.
792788
const auto spent = [=](const auto& input) NOEXCEPT
793789
{
794-
return input->metadata.spent && height > input->metadata.height;
790+
// Spends internal to a block are handled by block validation.
791+
// The lack of equality check here prevents conflict with self.
792+
return height > input->metadata.spender_height;
795793
};
796794

797795
return std::any_of(inputs_->begin(), inputs_->end(), spent);

‎test/chain/block.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ class accessor
7979
using block::is_first_non_coinbase;
8080
using block::is_extra_coinbases;
8181
using block::is_forward_reference;
82-
using block::is_extra_coinbases;
8382
using block::is_internal_double_spend;
8483
using block::is_invalid_merkle_root;
8584
using block::is_overweight;

‎test/chain/input.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_block_sequence_age_equal
245245
constexpr auto sequence_enabled_block_type_minimum = age;
246246
const input instance(point{}, {}, sequence_enabled_block_type_minimum);
247247
BOOST_REQUIRE(!instance.prevout);
248-
instance.metadata.height = 42;
249-
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.height + age, 0));
248+
instance.metadata.prevout_height = 42;
249+
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.prevout_height + age, 0));
250250
}
251251

252252
BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_block_type_sequence_age_above_minimum__false)
@@ -255,8 +255,8 @@ BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_block_type_sequence_age_
255255
constexpr auto sequence_enabled_block_type_minimum = sub1(age);
256256
const input instance(point{}, {}, sequence_enabled_block_type_minimum);
257257
BOOST_REQUIRE(!instance.prevout);
258-
instance.metadata.height = 42;
259-
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.height + age, 0));
258+
instance.metadata.prevout_height = 42;
259+
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.prevout_height + age, 0));
260260
}
261261

262262
BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_block_type_sequence_age_below_minimum__true)
@@ -265,8 +265,8 @@ BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_block_type_sequence_age_
265265
constexpr auto sequence_enabled_block_type_minimum = add1(age);
266266
const input instance(point{}, {}, sequence_enabled_block_type_minimum);
267267
BOOST_REQUIRE(!instance.prevout);
268-
instance.metadata.height = 42;
269-
BOOST_REQUIRE(instance.is_relative_locked(instance.metadata.height + age, 0));
268+
instance.metadata.prevout_height = 42;
269+
BOOST_REQUIRE(instance.is_relative_locked(instance.metadata.prevout_height + age, 0));
270270
}
271271

272272
BOOST_AUTO_TEST_CASE(input__is_relative_locked__disabled_block_type_sequence_age_below_minimum__false)
@@ -275,8 +275,8 @@ BOOST_AUTO_TEST_CASE(input__is_relative_locked__disabled_block_type_sequence_age
275275
constexpr auto sequence_disabled_block_type_minimum = bit_right<uint32_t>(relative_locktime_disabled_bit) | add1(age);
276276
const input instance(point{}, {}, sequence_disabled_block_type_minimum);
277277
BOOST_REQUIRE(!instance.prevout);
278-
instance.metadata.height = 42;
279-
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.height + age, 0));
278+
instance.metadata.prevout_height = 42;
279+
BOOST_REQUIRE(!instance.is_relative_locked(instance.metadata.prevout_height + age, 0));
280280
}
281281

282282
BOOST_AUTO_TEST_CASE(input__is_relative_locked__enabled_time_type_sequence_age_equals_minimum__false)

‎test/chain/transaction.cpp‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,7 +1126,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_immature__no_inputs__false)
11261126
BOOST_AUTO_TEST_CASE(transaction__is_immature__mature_genesis__true)
11271127
{
11281128
const input input{ { hash_digest{}, 42 }, {}, 0 };
1129-
input.metadata.height = 0;
1129+
input.metadata.prevout_height = 0;
11301130
input.metadata.coinbase = true;
11311131
const accessor instance
11321132
{
@@ -1142,7 +1142,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_immature__mature_genesis__true)
11421142
BOOST_AUTO_TEST_CASE(transaction__is_immature__premature_coinbase__true)
11431143
{
11441144
const input input{ { hash_digest{}, 42 }, {}, 0 };
1145-
input.metadata.height = 1;
1145+
input.metadata.prevout_height = 1;
11461146
input.metadata.coinbase = true;
11471147
const accessor instance
11481148
{
@@ -1158,7 +1158,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_immature__premature_coinbase__true)
11581158
BOOST_AUTO_TEST_CASE(transaction__is_immature__premature_non_coinbase__false)
11591159
{
11601160
const input input{ { hash_digest{}, 42 }, {}, 0 };
1161-
input.metadata.height = 1;
1161+
input.metadata.prevout_height = 1;
11621162
input.metadata.coinbase = false;
11631163
const accessor instance
11641164
{
@@ -1174,7 +1174,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_immature__premature_non_coinbase__false)
11741174
BOOST_AUTO_TEST_CASE(transaction__is_immature__mature_coinbase__false)
11751175
{
11761176
const input input{ { hash_digest{}, 42 }, {}, 0 };
1177-
input.metadata.height = 1;
1177+
input.metadata.prevout_height = 1;
11781178
input.metadata.coinbase = true;
11791179
const accessor instance
11801180
{
@@ -1190,7 +1190,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_immature__mature_coinbase__false)
11901190
BOOST_AUTO_TEST_CASE(transaction__is_immature__mature_non_coinbase__false)
11911191
{
11921192
const input input{ { hash_digest{}, 42 }, {}, 0 };
1193-
input.metadata.height = 1;
1193+
input.metadata.prevout_height = 1;
11941194
input.metadata.coinbase = false;
11951195
const accessor instance
11961196
{
@@ -1279,7 +1279,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_confirmed_double_spend__empty_inputs__false
12791279

12801280
BOOST_AUTO_TEST_CASE(transaction__is_confirmed_double_spend__default_inputs__false)
12811281
{
1282-
// input.metadata.spent defaults to true.
1282+
// input.metadata.spender_height defaults to max (spent above height).
12831283
const accessor instance
12841284
{
12851285
0,
@@ -1288,28 +1288,13 @@ BOOST_AUTO_TEST_CASE(transaction__is_confirmed_double_spend__default_inputs__fal
12881288
0
12891289
};
12901290

1291-
BOOST_REQUIRE(instance.is_confirmed_double_spend(42));
1292-
}
1293-
1294-
BOOST_AUTO_TEST_CASE(transaction__is_confirmed_double_spend__unspent_input__false)
1295-
{
1296-
const input input{ { hash_digest{}, 42 }, {}, 0 };
1297-
input.metadata.spent = false;
1298-
const accessor instance
1299-
{
1300-
0,
1301-
{ input },
1302-
{},
1303-
0
1304-
};
1305-
13061291
BOOST_REQUIRE(!instance.is_confirmed_double_spend(42));
13071292
}
13081293

13091294
BOOST_AUTO_TEST_CASE(transaction__is_confirmed_double_spend__spent_input__true)
13101295
{
13111296
const input input{ { hash_digest{}, 42 }, {}, 0 };
1312-
input.metadata.spent = true;
1297+
input.metadata.spender_height = 41;
13131298
const accessor instance
13141299
{
13151300
0,

0 commit comments

Comments
 (0)