Skip to content

Commit 6707961

Browse files
authored
[CHIA-2503] make the super set rule stricter (#19352)
* make the super set rule a stricter * use BundleCoinSpend bools rather than the flags to determine whether a spend supports fast forward and dedup
1 parent d0cdc44 commit 6707961

File tree

2 files changed

+77
-10
lines changed

2 files changed

+77
-10
lines changed

chia/_tests/core/mempool/test_mempool_manager.py

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ def make_test_conds(
207207
before_seconds_relative: Optional[int] = None,
208208
before_seconds_absolute: Optional[int] = None,
209209
cost: int = 0,
210-
spend_ids: list[bytes32] = [TEST_COIN_ID],
210+
spend_ids: list[tuple[bytes32, int]] = [(TEST_COIN_ID, 0)],
211211
) -> SpendBundleConditions:
212212
return SpendBundleConditions(
213213
[
@@ -230,9 +230,9 @@ def make_test_conds(
230230
[],
231231
[],
232232
[],
233-
0,
233+
flags,
234234
)
235-
for spend_id in spend_ids
235+
for spend_id, flags in spend_ids
236236
],
237237
0,
238238
uint32(height_absolute),
@@ -751,19 +751,25 @@ def mk_item(
751751
assert_height: Optional[int] = None,
752752
assert_before_height: Optional[int] = None,
753753
assert_before_seconds: Optional[int] = None,
754+
flags: list[int] = [],
754755
) -> MempoolItem:
755756
# we don't actually care about the puzzle and solutions for the purpose of
756757
# can_replace()
757-
spend_ids = []
758+
spend_ids: list[tuple[bytes32, int]] = []
758759
coin_spends = []
759760
bundle_coin_spends = {}
760-
for c in coins:
761+
if len(flags) < len(coins):
762+
flags.extend([0] * (len(coins) - len(flags)))
763+
for c, f in zip(coins, flags):
761764
coin_id = c.name()
762-
spend_ids.append(coin_id)
765+
spend_ids.append((coin_id, f))
763766
spend = make_spend(c, SerializedProgram.to(None), SerializedProgram.to(None))
764767
coin_spends.append(spend)
765768
bundle_coin_spends[coin_id] = BundleCoinSpend(
766-
coin_spend=spend, eligible_for_dedup=False, eligible_for_fast_forward=False, additions=[]
769+
coin_spend=spend,
770+
eligible_for_dedup=bool(f & ELIGIBLE_FOR_DEDUP),
771+
eligible_for_fast_forward=bool(f & ELIGIBLE_FOR_FF),
772+
additions=[],
767773
)
768774
spend_bundle = SpendBundle(coin_spends, G2Element())
769775
conds = make_test_conds(cost=cost, spend_ids=spend_ids)
@@ -817,6 +823,46 @@ def make_test_coins() -> list[Coin]:
817823
([mk_item(coins[0:2])], mk_item(coins[0:2], fee=10000000), True),
818824
# or if we spend the same coins with additional coins
819825
([mk_item(coins[0:2])], mk_item(coins[0:3], fee=10000000), True),
826+
# you're not allowed to clear the fast-forward or dedup flag. It's OK to set it
827+
# and leave it unchanged
828+
([mk_item(coins[0:2])], mk_item(coins[0:3], flags=[ELIGIBLE_FOR_DEDUP, 0, 0], fee=10000000), True),
829+
([mk_item(coins[0:2])], mk_item(coins[0:3], flags=[ELIGIBLE_FOR_FF, 0, 0], fee=10000000), True),
830+
# flag cleared
831+
([mk_item(coins[0:2], flags=[ELIGIBLE_FOR_DEDUP, 0])], mk_item(coins[0:3], fee=10000000), False),
832+
([mk_item(coins[0:2], flags=[ELIGIBLE_FOR_FF, 0])], mk_item(coins[0:3], fee=10000000), False),
833+
# unchanged
834+
(
835+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_DEDUP, 0])],
836+
mk_item(coins[0:3], flags=[ELIGIBLE_FOR_DEDUP, 0, 0], fee=10000000),
837+
True,
838+
),
839+
(
840+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_FF, 0])],
841+
mk_item(coins[0:3], flags=[ELIGIBLE_FOR_FF, 0, 0], fee=10000000),
842+
True,
843+
),
844+
# the spends are independent
845+
(
846+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_DEDUP, 0])],
847+
mk_item(coins[0:3], flags=[0, ELIGIBLE_FOR_DEDUP, 0], fee=10000000),
848+
False,
849+
),
850+
(
851+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_FF, 0])],
852+
mk_item(coins[0:3], flags=[0, ELIGIBLE_FOR_FF, 0], fee=10000000),
853+
False,
854+
),
855+
# the bits are independent
856+
(
857+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_DEDUP, 0])],
858+
mk_item(coins[0:3], flags=[ELIGIBLE_FOR_FF, 0, 0], fee=10000000),
859+
False,
860+
),
861+
(
862+
[mk_item(coins[0:2], flags=[ELIGIBLE_FOR_DEDUP, 0])],
863+
mk_item(coins[0:3], flags=[ELIGIBLE_FOR_FF, 0, 0], fee=10000000),
864+
False,
865+
),
820866
# FEE- AND FEE RATE RULES
821867
# if we're replacing two items, each paying a fee of 100, we need to
822868
# spend (at least) the same coins and pay at least 10000000 higher fee

chia/full_node/mempool_manager.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,13 @@ def can_replace(
861861
assert_height: Optional[uint32] = None
862862
assert_before_height: Optional[uint32] = None
863863
assert_before_seconds: Optional[uint64] = None
864+
# we don't allow replacing mempool items with new ones that remove
865+
# eligibility for dedup and fast-forward. Doing so could be abused by
866+
# denying such spends from operating as intended
867+
# collect all coins that are eligible for dedup and FF in the existing items
868+
existing_ff_spends: set[bytes32] = set()
869+
existing_dedup_spends: set[bytes32] = set()
870+
864871
for item in conflicting_items:
865872
conflicting_fees += item.fee
866873
conflicting_cost += item.cost
@@ -870,10 +877,14 @@ def can_replace(
870877
# bundle with AB with a higher fee. An attacker then replaces the bundle with just B with a higher
871878
# fee than AB therefore kicking out A altogether. The better way to solve this would be to keep a cache
872879
# of booted transactions like A, and retry them after they get removed from mempool due to a conflict.
873-
for coin in item.removals:
874-
if coin.name() not in removal_names:
875-
log.debug(f"Rejecting conflicting tx as it does not spend conflicting coin {coin.name()}")
880+
for coin_id, bcs in item.bundle_coin_spends.items():
881+
if coin_id not in removal_names:
882+
log.debug("Rejecting conflicting tx as it does not spend conflicting coin %s", coin_id)
876883
return False
884+
if bcs.eligible_for_fast_forward:
885+
existing_ff_spends.add(bytes32(coin_id))
886+
if bcs.eligible_for_dedup:
887+
existing_dedup_spends.add(bytes32(coin_id))
877888

878889
assert_height = optional_max(assert_height, item.assert_height)
879890
assert_before_height = optional_min(assert_before_height, item.assert_before_height)
@@ -919,5 +930,15 @@ def can_replace(
919930
)
920931
return False
921932

933+
if len(existing_ff_spends) > 0 or len(existing_dedup_spends) > 0:
934+
for coin_id, bcs in new_item.bundle_coin_spends.items():
935+
if not bcs.eligible_for_fast_forward and coin_id in existing_ff_spends:
936+
log.debug("Rejecting conflicting tx due to changing ELIGIBLE_FOR_FF of coin spend %s", coin_id)
937+
return False
938+
939+
if not bcs.eligible_for_dedup and coin_id in existing_dedup_spends:
940+
log.debug("Rejecting conflicting tx due to changing ELIGIBLE_FOR_DEDUP of coin spend %s", coin_id)
941+
return False
942+
922943
log.info(f"Replacing conflicting tx in mempool. New tx fee: {new_item.fee}, old tx fees: {conflicting_fees}")
923944
return True

0 commit comments

Comments
 (0)