Skip to content

Commit cbf10a7

Browse files
committed
refactor(verification): reorganize min_height verification
1 parent a25f3a7 commit cbf10a7

28 files changed

+118
-63
lines changed

hathor/consensus/transaction_consensus.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ def check_conflicts(self, tx: Transaction) -> None:
310310

311311
# If we got here, either it was a tie or we won.
312312
# So, let's void the conflict txs.
313-
for conflict_tx in conflict_list:
313+
for conflict_tx in sorted(conflict_list, key=lambda x: x.timestamp, reverse=True):
314314
self.mark_as_voided(conflict_tx)
315315

316316
if not tie_list:

hathor/manager.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -914,12 +914,6 @@ def push_tx(self, tx: Transaction, allow_non_standard_script: bool = False,
914914
if not tx_from_lib.is_standard(max_output_script_size, not allow_non_standard_script):
915915
raise NonStandardTxError('Transaction is non standard.')
916916

917-
# Validate tx.
918-
try:
919-
self.verification_service.verify(tx)
920-
except TxValidationError as e:
921-
raise InvalidNewTransaction(str(e))
922-
923917
self.propagate_tx(tx, fails_silently=False)
924918

925919
def propagate_tx(self, tx: BaseTransaction, fails_silently: bool = True) -> bool:

hathor/reward_lock/reward_lock.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,16 @@ def get_spent_reward_locked_info(tx: 'Transaction', storage: 'VertexStorageProto
4242
"""Check if any input block reward is locked, returning the locked information if any, or None if they are all
4343
unlocked."""
4444
from hathor.transaction.transaction import RewardLockedInfo
45+
best_height = get_minimum_best_height(storage)
4546
for blk in iter_spent_rewards(tx, storage):
46-
needed_height = _spent_reward_needed_height(blk, storage)
47+
needed_height = _spent_reward_needed_height(blk, best_height)
4748
if needed_height > 0:
4849
return RewardLockedInfo(blk.hash, needed_height)
4950
return None
5051

5152

52-
def _spent_reward_needed_height(block: Block, storage: 'VertexStorageProtocol') -> int:
53-
""" Returns height still needed to unlock this `block` reward: 0 means it's unlocked."""
53+
def get_minimum_best_height(storage: 'VertexStorageProtocol') -> int:
54+
"""Return the height of the current best block that shall be used for `min_height` verification."""
5455
import math
5556

5657
# omitting timestamp to get the current best block, this will usually hit the cache instead of being slow
@@ -61,6 +62,11 @@ def _spent_reward_needed_height(block: Block, storage: 'VertexStorageProtocol')
6162
blk = storage.get_block(tip)
6263
best_height = min(best_height, blk.get_height())
6364
assert isinstance(best_height, int)
65+
return best_height
66+
67+
68+
def _spent_reward_needed_height(block: Block, best_height: int) -> int:
69+
""" Returns height still needed to unlock this `block` reward: 0 means it's unlocked."""
6470
spent_height = block.get_height()
6571
spend_blocks = best_height - spent_height
6672
settings = get_global_settings()

hathor/simulator/utils.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@
2020
from hathor.types import Address, VertexId
2121

2222

23-
def gen_new_tx(manager: HathorManager, address: str, value: int, verify: bool = True) -> Transaction:
23+
def gen_new_tx(manager: HathorManager, address: str, value: int) -> Transaction:
2424
"""
2525
Generate and return a new transaction.
2626
2727
Args:
2828
manager: the HathorManager to generate the transaction for
2929
address: an address for the transaction's output
3030
value: a value for the transaction's output
31-
verify: whether to verify the generated transaction
3231
3332
Returns: the generated transaction.
3433
"""
@@ -48,8 +47,6 @@ def gen_new_tx(manager: HathorManager, address: str, value: int, verify: bool =
4847
tx.weight = 1
4948
tx.parents = manager.get_new_tx_parents(tx.timestamp)
5049
manager.cpu_mining_service.resolve(tx)
51-
if verify:
52-
manager.verification_service.verify(tx)
5350
return tx
5451

5552

@@ -111,7 +108,6 @@ def add_new_block(
111108
if signal_bits is not None:
112109
block.signal_bits = signal_bits
113110
manager.cpu_mining_service.resolve(block)
114-
manager.verification_service.validate_full(block)
115111
if propagate:
116112
manager.propagate_tx(block, fails_silently=False)
117113
if advance_clock:

hathor/transaction/base_transaction.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -624,13 +624,14 @@ def get_metadata(self, *, force_reload: bool = False, use_storage: bool = True)
624624
# happens include generating new mining blocks and some tests
625625
height = self.calculate_height() if self.storage else None
626626
score = self.weight if self.is_genesis else 0
627+
min_height = 0 if self.is_genesis else None
627628

628629
metadata = TransactionMetadata(
629630
hash=self._hash,
630631
accumulated_weight=self.weight,
631632
height=height,
632633
score=score,
633-
min_height=0,
634+
min_height=min_height
634635
)
635636
self._metadata = metadata
636637
if not metadata.hash:
@@ -713,8 +714,9 @@ def update_initial_metadata(self, *, save: bool = True) -> None:
713714
"""
714715
self._update_height_metadata()
715716
self._update_parents_children_metadata()
716-
self._update_reward_lock_metadata()
717+
self.update_reward_lock_metadata()
717718
self._update_feature_activation_bit_counts()
719+
self._update_initial_accumulated_weight()
718720
if save:
719721
assert self.storage is not None
720722
self.storage.save_transaction(self, only_metadata=True)
@@ -724,10 +726,13 @@ def _update_height_metadata(self) -> None:
724726
meta = self.get_metadata()
725727
meta.height = self.calculate_height()
726728

727-
def _update_reward_lock_metadata(self) -> None:
729+
def update_reward_lock_metadata(self) -> None:
728730
"""Update the txs/block min_height metadata."""
729731
metadata = self.get_metadata()
730-
metadata.min_height = self.calculate_min_height()
732+
min_height = self.calculate_min_height()
733+
if metadata.min_height is not None:
734+
assert metadata.min_height == min_height
735+
metadata.min_height = min_height
731736

732737
def _update_parents_children_metadata(self) -> None:
733738
"""Update the txs/block parent's children metadata."""
@@ -749,6 +754,11 @@ def _update_feature_activation_bit_counts(self) -> None:
749754
# This method lazily calculates and stores the value in metadata
750755
self.get_feature_activation_bit_counts()
751756

757+
def _update_initial_accumulated_weight(self) -> None:
758+
"""Update the vertex initial accumulated_weight."""
759+
metadata = self.get_metadata()
760+
metadata.accumulated_weight = self.weight
761+
752762
def update_timestamp(self, now: int) -> None:
753763
"""Update this tx's timestamp
754764

hathor/transaction/storage/transaction_storage.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from hathor.pubsub import PubSubManager
3232
from hathor.transaction.base_transaction import BaseTransaction, TxOutput
3333
from hathor.transaction.block import Block
34+
from hathor.transaction.exceptions import RewardLocked
3435
from hathor.transaction.storage.exceptions import (
3536
TransactionDoesNotExist,
3637
TransactionIsNotABlock,
@@ -49,6 +50,7 @@
4950
from hathor.transaction.transaction import Transaction
5051
from hathor.transaction.transaction_metadata import TransactionMetadata
5152
from hathor.types import VertexId
53+
from hathor.verification.transaction_verifier import TransactionVerifier
5254

5355
cpu = get_cpu_profiler()
5456

@@ -1097,10 +1099,11 @@ def compute_transactions_that_became_invalid(self, new_best_height: int) -> list
10971099
from hathor.transaction.validation_state import ValidationState
10981100
to_remove: list[BaseTransaction] = []
10991101
for tx in self.iter_mempool_from_best_index():
1100-
tx_min_height = tx.get_metadata().min_height
1101-
assert tx_min_height is not None
1102-
# We use +1 here because a tx is valid if it can be confirmed by the next block
1103-
if new_best_height + 1 < tx_min_height:
1102+
try:
1103+
TransactionVerifier.verify_reward_locked_for_height(
1104+
tx, new_best_height, assert_min_height_verification=False
1105+
)
1106+
except RewardLocked:
11041107
tx.set_validation(ValidationState.INVALID)
11051108
to_remove.append(tx)
11061109
return to_remove

hathor/verification/transaction_verifier.py

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from hathor.daa import DifficultyAdjustmentAlgorithm
1717
from hathor.profiler import get_cpu_profiler
1818
from hathor.reward_lock import get_spent_reward_locked_info
19+
from hathor.reward_lock.reward_lock import get_minimum_best_height
1920
from hathor.transaction import BaseTransaction, Transaction, TxInput
2021
from hathor.transaction.exceptions import (
2122
ConflictingInputs,
@@ -37,7 +38,6 @@
3738
from hathor.transaction.transaction import TokenInfo
3839
from hathor.transaction.util import get_deposit_amount, get_withdraw_amount
3940
from hathor.types import TokenUid, VertexId
40-
from hathor.util import not_none
4141

4242
cpu = get_cpu_profiler()
4343

@@ -142,12 +142,51 @@ def verify_script(self, *, tx: Transaction, input_tx: TxInput, spent_tx: BaseTra
142142
raise InvalidInputData(e) from e
143143

144144
def verify_reward_locked(self, tx: Transaction) -> None:
145-
"""Will raise `RewardLocked` if any reward is spent before the best block height is enough, considering only
146-
the block rewards spent by this tx itself, and not the inherited `min_height`."""
147-
info = get_spent_reward_locked_info(tx, not_none(tx.storage))
145+
"""Will raise `RewardLocked` if any reward is spent before the best block height is enough, considering both
146+
the block rewards spent by this tx itself, and the inherited `min_height`."""
147+
assert tx.storage is not None
148+
best_height = get_minimum_best_height(tx.storage)
149+
self.verify_reward_locked_for_height(tx, best_height)
150+
151+
@staticmethod
152+
def verify_reward_locked_for_height(
153+
tx: Transaction,
154+
best_height: int,
155+
*,
156+
assert_min_height_verification: bool = True
157+
) -> None:
158+
"""
159+
Will raise `RewardLocked` if any reward is spent before the best block height is enough, considering both
160+
the block rewards spent by this tx itself, and the inherited `min_height`.
161+
162+
Args:
163+
tx: the transaction to be verified.
164+
best_height: the height of the best chain to be used for verification.
165+
assert_min_height_verification: whether the inherited `min_height` verification must pass.
166+
167+
Note: for verification of new transactions, `assert_min_height_verification` must be `True`. This
168+
verification is always expected to pass for new txs, as a failure would mean one of its dependencies would
169+
have failed too. So an `AssertionError` is raised if it fails.
170+
171+
However, when txs are being re-verified for Reward Lock during a reorg, it's possible that txs may fail
172+
their inherited `min_height` verification. So in that case `assert_min_height_verification` is `False`,
173+
and a normal `RewardLocked` exception is raised instead.
174+
"""
175+
assert tx.storage is not None
176+
info = get_spent_reward_locked_info(tx, tx.storage)
148177
if info is not None:
149178
raise RewardLocked(f'Reward {info.block_hash.hex()} still needs {info.blocks_needed} to be unlocked.')
150179

180+
meta = tx.get_metadata()
181+
assert meta.min_height is not None
182+
# We use +1 here because a tx is valid if it can be confirmed by the next block
183+
if best_height + 1 < meta.min_height:
184+
if assert_min_height_verification:
185+
raise AssertionError('a new tx should never be invalid by its inherited min_height.')
186+
raise RewardLocked(
187+
f'Tx {tx.hash_hex} has min_height={meta.min_height}, but the best_height={best_height}.'
188+
)
189+
151190
def verify_number_of_inputs(self, tx: Transaction) -> None:
152191
"""Verify number of inputs is in a valid range"""
153192
if len(tx.inputs) > self._settings.MAX_NUM_INPUTS:

hathor/vertex_handler/vertex_handler.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,13 @@ def _validate_vertex(
150150
return False
151151

152152
if not metadata.validation.is_fully_connected():
153+
# TODO: Remove this from here after a refactor in metadata initialization
154+
vertex.update_reward_lock_metadata()
153155
try:
154156
self._verification_service.validate_full(vertex, reject_locked_reward=reject_locked_reward)
155157
except HathorError as e:
156158
if not fails_silently:
157-
raise InvalidNewTransaction('full validation failed') from e
159+
raise InvalidNewTransaction(f'full validation failed: {str(e)}') from e
158160
self._log.warn('on_new_tx(): full validation failed', tx=vertex.hash_hex, exc_info=True)
159161
return False
160162

hathor/wallet/resources/send_tokens.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ def _render_POST_thread(self, values: dict[str, Any], request: Request) -> Union
127127
weight = self.manager.daa.minimum_tx_weight(tx)
128128
tx.weight = weight
129129
self.manager.cpu_mining_service.resolve(tx)
130+
tx.update_reward_lock_metadata()
130131
self.manager.verification_service.verify(tx)
131132
return tx
132133

hathor/wallet/resources/thin_wallet/send_tokens.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ def _should_stop():
270270
if context.should_stop_mining_thread:
271271
raise CancelledError()
272272
context.tx.update_hash()
273+
context.tx.update_reward_lock_metadata()
273274
self.manager.verification_service.verify(context.tx)
274275
return context
275276

tests/consensus/test_consensus.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ def test_revert_block_high_weight(self) -> None:
8181
b0 = tb0.generate_mining_block(manager.rng, storage=manager.tx_storage)
8282
b0.weight = 10
8383
manager.cpu_mining_service.resolve(b0)
84-
manager.verification_service.verify(b0)
8584
manager.propagate_tx(b0, fails_silently=False)
8685

8786
b1 = add_new_block(manager, advance_clock=15)
@@ -143,7 +142,6 @@ def test_dont_revert_block_low_weight(self) -> None:
143142
b0 = manager.generate_mining_block()
144143
b0.parents = [blocks[-1].hash, conflicting_tx.hash, conflicting_tx.parents[0]]
145144
manager.cpu_mining_service.resolve(b0)
146-
manager.verification_service.verify(b0)
147145
manager.propagate_tx(b0, fails_silently=False)
148146

149147
b1 = add_new_block(manager, advance_clock=15)
@@ -199,7 +197,6 @@ def test_dont_revert_block_high_weight_transaction_verify_other(self) -> None:
199197
b0 = tb0.generate_mining_block(manager.rng, storage=manager.tx_storage)
200198
b0.weight = 10
201199
manager.cpu_mining_service.resolve(b0)
202-
manager.verification_service.verify(b0)
203200
manager.propagate_tx(b0, fails_silently=False)
204201

205202
b1 = add_new_block(manager, advance_clock=15)
@@ -253,7 +250,6 @@ def test_dont_revert_block_high_weight_verify_both(self) -> None:
253250
b0.parents = [b0.parents[0], conflicting_tx.hash, conflicting_tx.parents[0]]
254251
b0.weight = 10
255252
manager.cpu_mining_service.resolve(b0)
256-
manager.verification_service.verify(b0)
257253
manager.propagate_tx(b0, fails_silently=False)
258254

259255
b1 = add_new_block(manager, advance_clock=15)

tests/event/test_event_reorg.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ def test_reorg_events(self) -> None:
3636
b0 = tb0.generate_mining_block(self.manager.rng, storage=self.manager.tx_storage, address=BURN_ADDRESS)
3737
b0.weight = 10
3838
self.manager.cpu_mining_service.resolve(b0)
39-
self.manager.verification_service.verify(b0)
4039
self.manager.propagate_tx(b0, fails_silently=False)
4140
self.log.debug('reorg block propagated')
4241
self.run_to_completion()

tests/feature_activation/test_feature_simulation.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ def test_feature(self) -> None:
228228
non_signaling_block = manager.generate_mining_block()
229229
manager.cpu_mining_service.resolve(non_signaling_block)
230230
non_signaling_block.signal_bits = 0b10
231+
non_signaling_block.update_reward_lock_metadata()
231232

232233
with pytest.raises(BlockMustSignalError):
233234
manager.verification_service.verify(non_signaling_block)

tests/p2p/test_sync.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ def _add_new_tx(self, address: str, value: int) -> Transaction:
4444
tx.weight = 10
4545
tx.parents = self.manager1.get_new_tx_parents()
4646
self.manager1.cpu_mining_service.resolve(tx)
47-
self.manager1.verification_service.verify(tx)
4847
self.manager1.propagate_tx(tx)
4948
self.clock.advance(10)
5049
return tx
@@ -61,7 +60,6 @@ def _add_new_transactions(self, num_txs: int) -> list[Transaction]:
6160
def _add_new_block(self, propagate: bool = True) -> Block:
6261
block: Block = self.manager1.generate_mining_block()
6362
self.assertTrue(self.manager1.cpu_mining_service.resolve(block))
64-
self.manager1.verification_service.verify(block)
6563
self.manager1.on_new_tx(block, propagate_to_peers=propagate)
6664
self.clock.advance(10)
6765
return block

tests/p2p/test_sync_mempool.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ def _add_new_tx(self, address: str, value: int) -> Transaction:
3636
tx.weight = 10
3737
tx.parents = self.manager1.get_new_tx_parents()
3838
self.manager1.cpu_mining_service.resolve(tx)
39-
self.manager1.verification_service.verify(tx)
4039
self.manager1.propagate_tx(tx)
4140
self.clock.advance(10)
4241
return tx
@@ -53,7 +52,6 @@ def _add_new_transactions(self, num_txs: int) -> list[Transaction]:
5352
def _add_new_block(self, propagate: bool = True) -> Block:
5453
block: Block = self.manager1.generate_mining_block()
5554
self.assertTrue(self.manager1.cpu_mining_service.resolve(block))
56-
self.manager1.verification_service.verify(block)
5755
self.manager1.on_new_tx(block, propagate_to_peers=propagate)
5856
self.clock.advance(10)
5957
return block

tests/p2p/test_sync_v2.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ def test_receiving_tips_limit(self) -> None:
276276
# Custom tx generator that generates tips
277277
parents = manager1.get_new_tx_parents(manager1.tx_storage.latest_timestamp)
278278

279-
def custom_gen_new_tx(manager: HathorManager, _address: str, value: int, verify: bool = True) -> Transaction:
279+
def custom_gen_new_tx(manager: HathorManager, _address: str, value: int) -> Transaction:
280280
outputs = []
281281
# XXX: burn address guarantees that this output will not be used as input for any following transactions
282282
# XXX: reduce value to make sure we can generate more transactions, otherwise it will spend a linear random
@@ -294,8 +294,6 @@ def custom_gen_new_tx(manager: HathorManager, _address: str, value: int, verify:
294294
# XXX: fixed parents is the final requirement to make all the generated new tips
295295
tx.parents = parents
296296
manager.cpu_mining_service.resolve(tx)
297-
if verify:
298-
manager.verification_service.verify(tx)
299297
return tx
300298

301299
# Generate 100 tx-tips in mempool.

tests/resources/transaction/test_mining.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def test_get_block_template_with_address(self):
3838
'accumulated_weight': 1.0,
3939
'score': 0,
4040
'height': 1,
41-
'min_height': 0,
41+
'min_height': None,
4242
'first_block': None,
4343
'feature_activation_bit_counts': None
4444
},
@@ -71,7 +71,7 @@ def test_get_block_template_without_address(self):
7171
'accumulated_weight': 1.0,
7272
'score': 0,
7373
'height': 1,
74-
'min_height': 0,
74+
'min_height': None,
7575
'first_block': None,
7676
'feature_activation_bit_counts': None
7777
},

0 commit comments

Comments
 (0)