Skip to content

Commit

Permalink
test: Set -limitdescendantcount to match viable iteration limit
Browse files Browse the repository at this point in the history
The original upstream commit looped sufficient times to reach the
default value of `-limitdescendantcount`. However, it hard-coded the
default of 1000, while also setting `-maxorphantx=1000`, leading to the
erroneous belief that the former was derived from the latter (instead of
what we now believe to be the reverse).

When the value was decreased from 1000 to 120 (to account for ZIP 317
fees being larger, and lower block subsidy), `-maxorphantx` was changed
but `-limitdescendantcount` was not set. This meant that the loop that
creates the descendant tree would succeed one extra time, creating 121
transactions (because the descendant limit was still the default of
1000).

Our hypothesis is that during the next phase of the test, something
happens that causes these transactions to be treated as orphans, and the
`-maxorphantx=120` setting was preventing them from being synced between
nodes, causing timeouts that would then make the test more vulnerable to
the GCP pods backing Tekton CI being evicted. We haven't confirmed this,
but this commit fixes a clear bug so we are hoping this is sufficient to
fix the non-deterministic test failures.

Fixes zcash#6723.

Co-authored-by: Kris Nuttycombe <kris@nutty.land>
  • Loading branch information
str4d and nuttycom committed Jun 23, 2023
1 parent 57c8385 commit 4000302
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions qa/rpc-tests/mempool_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ def satoshi_round(amount):
return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)

class MempoolPackagesTest(BitcoinTestFramework):
maxorphantx = 120
limitdescendantcount = 120

def setup_network(self):
base_args = [
'-limitdescendantcount=%d' % (self.limitdescendantcount,),
'-minrelaytxfee=0',
'-maxorphantx=%d' % (self.maxorphantx,),
'-maxorphantx=%d' % (self.limitdescendantcount,),
'-debug',
'-allowdeprecated=getnewaddress',
]
Expand Down Expand Up @@ -145,19 +146,23 @@ def run_test(self):
for i in range(10):
transaction_package.append({'txid': txid, 'vout': i, 'amount': sent_value})

for i in range(self.maxorphantx):
errored_too_large = False
for i in range(self.limitdescendantcount):
utxo = transaction_package.pop(0)
try:
(txid, sent_value) = self.chain_transaction(self.nodes[0], utxo['txid'], utxo['vout'], utxo['amount'], fee, 10)
for j in range(10):
transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value})
if i == self.maxorphantx-2:
if i == self.limitdescendantcount-2:
mempool = self.nodes[0].getrawmempool(True)
assert_equal(mempool[parent_transaction]['descendantcount'], self.maxorphantx)
assert_equal(mempool[parent_transaction]['descendantcount'], self.limitdescendantcount)
except JSONRPCException as e:
print(e.error['message'])
assert_equal(i, self.maxorphantx-1)
assert_equal(i, self.limitdescendantcount-1)
print("tx that would create too large descendant package successfully rejected")
errored_too_large = True
assert errored_too_large
assert_equal(len(transaction_package), self.limitdescendantcount * (10 - 1))

# TODO: check that node1's mempool is as expected

Expand Down

0 comments on commit 4000302

Please sign in to comment.