[Tests] Use blockmaxweight where tests previously had blockmaxsize

conscott commented at 1:23 pm on March 26, 2018: contributor

Fix for #12768: -blockmaxsize has been removed, but some tests were using this feature, so update with -blockmaxweight

in test/functional/feature_fee_estimation.py:141 in 321c448163 outdated

139+                                      ["-blockmaxweight=32000", "-maxorphantx=1000"]])
140         # Use node0 to mine blocks for input splitting
141         # Node1 mines small blocks but that are bigger than the expected transaction rate.
142         # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes,
143-        # (17k is room enough for 110 or so transactions)
144+        # (68k weight is room enough for 120 or so transactions)

conscott commented at 1:24 pm on March 26, 2018:

I have verified these numbers are still correct.

jnewbery commented at 3:04 pm on March 26, 2018:

Good! Is this test using segwit or legacy transactions? I assume that would have an impact on these numbers?

conscott commented at 3:37 pm on March 26, 2018:

Legacy P2SH - so just a simple WITNESS_SCALE_FACTOR for all txs.

fanquake added the label Tests on Mar 26, 2018

fanquake commented at 1:25 pm on March 26, 2018: member

cc @ajtowns

conscott commented at 1:27 pm on March 26, 2018: contributor

Request review from @jnewbery - since he posted the issue.

laanwj commented at 1:34 pm on March 26, 2018: member

Needs rebase after #12756 (if still an issue).

conscott force-pushed on Mar 26, 2018

in test/functional/feature_maxuploadtarget.py:37 in 5e1709628c outdated

34+
35     def set_test_params(self):
36         self.setup_clean_chain = True
37         self.num_nodes = 1
38-        self.extra_args = [["-maxuploadtarget=800"]]
39+        self.extra_args = [["-maxuploadtarget=800", "-blockmaxweight=3996000"]]

conscott commented at 1:45 pm on March 26, 2018:

May just remove these, since it’s the default, but the limit was explicitly added in the original test.

jnewbery commented at 2:55 pm on March 26, 2018:

I agree that we should just remove this - there’s no point in explicitly setting config to the default value (in fact it just distracts from the purpose of the test).

conscott commented at 1:48 pm on March 26, 2018: contributor

Rebased.

in test/functional/feature_fee_estimation.py:140 in 5e1709628c outdated

137-                                      ["-maxorphantx=1000"]])
138+                                      ["-blockmaxweight=68000", "-maxorphantx=1000"],
139+                                      ["-blockmaxweight=32000", "-maxorphantx=1000"]])
140         # Use node0 to mine blocks for input splitting
141         # Node1 mines small blocks but that are bigger than the expected transaction rate.
142         # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes,

jnewbery commented at 3:04 pm on March 26, 2018:

This comment can be updated to say “CreateNewBlock code starts counting block weight at 4,000 weight”

jnewbery commented at 3:05 pm on March 26, 2018: member

utACK. I agree that -blockmaxweight=3996000 should be removed. One request to update comments inline.

promag commented at 3:13 pm on March 26, 2018: member

PR title and commit message are outdated, there is no such thing as blockmaxsize 😄

conscott force-pushed on Mar 26, 2018

conscott renamed this:
~~[Tests] Replace deprecated blockmaxsize with blockmaxweight~~
[Tests] Use blockmaxweight where tests previously had blockmaxsize
on Mar 26, 2018

in test/functional/feature_fee_estimation.py:143 in 0868bd1592 outdated

142-        # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes,
143-        # (17k is room enough for 110 or so transactions)
144+        # NOTE: the CreateNewBlock code starts counting block weight at 4,000 weight,
145+        # (68k weight is room enough for 120 or so transactions)
146         # Node2 is a stingy miner, that
147         # produces too small blocks (room for only 55 or so transactions)

jnewbery commented at 3:43 pm on March 26, 2018:

Is this number still correct?

conscott commented at 3:52 pm on March 26, 2018:

Yes. I tested this as well.

in test/functional/feature_pruning.py:34 in 0868bd1592 outdated

30@@ -31,14 +31,14 @@ def set_test_params(self):
31 
32         # Create nodes 0 and 1 to mine.
33         # Create node 2 to test pruning.
34-        self.full_node_default_args = ["-maxreceivebuffer=20000", "-checkblocks=5", "-limitdescendantcount=100", "-limitdescendantsize=5000", "-limitancestorcount=100", "-limitancestorsize=5000" ]
35+        self.full_node_default_args = ["-maxreceivebuffer=20000", "-blockmaxweight=3996000", "-checkblocks=5", "-limitdescendantcount=100", "-limitdescendantsize=5000", "-limitancestorcount=100", "-limitancestorsize=5000"]

jnewbery commented at 3:44 pm on March 26, 2018:

I think this change can be removed

in test/functional/feature_pruning.py:41 in 0868bd1592 outdated

39                            self.full_node_default_args,
40                            ["-maxreceivebuffer=20000", "-prune=550"],
41-                           ["-maxreceivebuffer=20000"],
42-                           ["-maxreceivebuffer=20000"],
43+                           ["-maxreceivebuffer=20000", "-blockmaxweight=3996000"],
44+                           ["-maxreceivebuffer=20000", "-blockmaxweight=3996000"],

jnewbery commented at 3:44 pm on March 26, 2018:

I think this change can be removed

[Tests] Use blockmaxweight where tests previously had blockmaxsize b466f6be95

conscott force-pushed on Mar 26, 2018

MarcoFalke commented at 4:03 pm on March 26, 2018: member

utACK b466f6be959b25f3a07cede9f03563ed0bbda0f

jnewbery commented at 4:12 pm on March 26, 2018: member

Don’t we persist the mempool to disk?

Yes, unless -persistmempool=0 is set.

jnewbery commented at 4:29 pm on March 26, 2018: member

Tested ACK b466f6be959b25f3a07cede9f03563ed0bbda0fa

promag commented at 5:20 pm on March 26, 2018: member

utACK b466f6b.

ajtowns commented at 10:32 pm on March 26, 2018: member

utACK b466f6be959b25f3a07cede9f03563ed0bbda0fa

laanwj merged this on Mar 29, 2018

laanwj closed this on Mar 29, 2018

laanwj referenced this in commit 490644d29e on Mar 29, 2018

conscott deleted the branch on Jul 31, 2018

MarcoFalke locked this on Sep 8, 2021

[Tests] Use blockmaxweight where tests previously had blockmaxsize #12790