test: Add various low-level p2p tests #18628

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2004-qaP2Pvar changing 5 files +63 −3

MarcoFalke commented at 1:34 AM on April 14, 2020: member

No description provided.
fanquake added the label Tests on Apr 14, 2020
test: Add various low-level p2p tests fa4c29bc1d
MarcoFalke force-pushed on Apr 14, 2020

DrahtBot commented at 2:20 AM on April 14, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18610 (scripted-diff: test: replace command with msgtype (naming) by theStack)
#18498 (test: enable opening partial p2p connections where useful by jonatack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Coverage

Coverage	Change (pull 18628, e1f1b0cb6567131f86a2c370d31dc6674110896a)	Reference (master, 6110ae8326c74704c9e105deca725f2411395969)
Lines	+0.0618 %	90.3205 %
Functions	+0.0435 %	85.9983 %
Branches	+0.0408 %	51.8032 %

<sup>Updated at: 2020-04-14T02:20:17.491254.</sup>

practicalswift commented at 3:20 PM on April 14, 2020: contributor

Concept ACK: More coverage is better

in test/functional/p2p_filter.py:67 in fa4c29bc1d

  59 | @@ -60,8 +60,13 @@ def skip_test_if_missing_module(self):
  60 |          self.skip_if_no_wallet()
  61 |  
  62 |      def run_test(self):
  63 | -        self.log.info('Add filtered P2P connection to the node')
  64 |          filter_node = self.nodes[0].add_p2p_connection(FilterNode())
  65 | +
  66 | +        self.log.info('Check that too large filter is rejected')
  67 | +        with self.nodes[0].assert_debug_log(['Misbehaving']):
  68 | +            filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=51, nTweak=0, nFlags=1))

theStack commented at 6:48 PM on April 14, 2020:

Could also test the other filter size limit, i.e. one with a data size of > 36000 bytes.

MarcoFalke commented at 9:32 PM on April 14, 2020:

Will leave as-is for now to not invalidate reviews. Can be done as a follow-up

in test/functional/p2p_invalid_tx.py:164 in fa4c29bc1d

 159 | +            node.p2p.send_txs_and_test(orphan_tx_pool, node, success=False)
 160 | +
 161 | +        rejected_parent = CTransaction()
 162 | +        rejected_parent.vin.append(CTxIn(outpoint=COutPoint(tx_orphan_2_invalid.sha256, 0)))
 163 | +        rejected_parent.vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=SCRIPT_PUB_KEY_OP_TRUE))
 164 | +        rejected_parent.rehash()

theStack commented at 7:09 PM on April 14, 2020:

nit: could ~also~ instead use the method calc_sha256() here to be consistent with all other uses above

MarcoFalke commented at 9:31 PM on April 14, 2020:

I uesed calc_sha256 when later tx.sha256 was accessed and rehash when later tx.hash was accessed

theStack commented at 7:18 PM on April 14, 2020: member

Concept ACK
jonatack commented at 8:59 PM on April 14, 2020: member

ACK fa4c29bc1d242
MarcoFalke merged this on Apr 15, 2020
MarcoFalke closed this on Apr 15, 2020
MarcoFalke deleted the branch on Apr 15, 2020
sidhujag referenced this in commit 8288222c47 on Apr 15, 2020
MarcoFalke referenced this in commit 4ad6144ed0 on Apr 21, 2020
sidhujag referenced this in commit f398e24e39 on Apr 23, 2020
Fabcien referenced this in commit 8c16294155 on Jan 15, 2021
deadalnix referenced this in commit 197d07192c on Jan 19, 2021
random-zebra referenced this in commit bffe509aed on Jun 28, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18672 test: add further BIP37 size limit checks to p2p_filter.py