test: perturb anchors.dat to test error during initialization #26314

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2022-10-test-perturb-anchors changing 1 files +13 −5

brunoerg commented at 5:33 PM on October 14, 2022: contributor

Got some inspiration from feature_init. This PR tests whether perturbing anchors.dat doesn't throw any error during initialization.

https://github.com/bitcoin/bitcoin/blob/3f1f5f6f1ec49d0fb2acfddec4021b3582ba0553/src/addrdb.cpp#L223-L235
DrahtBot added the label Tests on Oct 14, 2022
test: perturb anchors.dat to test it doesn't throw an error during initialization 33fdfc7986

in test/functional/feature_anchors.py:80 in 9eab07ecd8 outdated

  78 | +
  79 | +        self.log.info("Perturb anchors.dat to test it doesn't throw an error during initialization")
  80 | +        with open(node_anchors_path, "wb") as out_file_handler:
  81 | +            tweaked_contents = bytearray(anchors)
  82 | +            tweaked_contents[10:20] = b'1' * 25
  83 | +            out_file_handler.write(bytes(tweaked_contents))

maflcko commented at 7:41 AM on October 17, 2022:

Maybe writing a single byte b'a' or so achieves the same?

brunoerg commented at 1:18 PM on October 17, 2022:

Yes, it achieves the same, just tested here manually btw... Do you think it's best to change it to modify a single byte ? e.g.:

diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py
index aa80b9f1d..322b6b177 100755
--- a/test/functional/feature_anchors.py
+++ b/test/functional/feature_anchors.py
@@ -76,7 +76,7 @@ class AnchorsTest(BitcoinTestFramework):
         self.log.info("Perturb anchors.dat to test it doesn't throw an error during initialization")
         with open(node_anchors_path, "wb") as out_file_handler:
             tweaked_contents = bytearray(anchors)
-            tweaked_contents[10:20] = b'1' * 25
+            tweaked_contents[35:35] = b'1'
             out_file_handler.write(bytes(tweaked_contents))

brunoerg force-pushed on Oct 28, 2022
brunoerg commented at 6:23 PM on October 28, 2022: contributor
Force-pushed:
- Perturb file in a lighter way (e.g. single byte)
- Added a assert_debug_log to ensure that 0 anchors will be tried for connection.
in test/functional/feature_anchors.py:80 in 33fdfc7986
```
  80 | -        self.start_node(0)
  81 | +        self.log.info("Perturb anchors.dat to test it doesn't throw an error during initialization")
  82 | +        with self.nodes[0].assert_debug_log(["0 block-relay-only anchors will be tried for connections."]):
  83 | +            with open(node_anchors_path, "wb") as out_file_handler:
  84 | +                tweaked_contents = bytearray(anchors)
  85 | +                tweaked_contents[20:20] = b'1'
```
maflcko commented at 9:01 AM on October 29, 2022:

how do you know that the existing byte is not b'1' and this is not a no-op. Even if it isn't a no-op, this may still pass de-serialization?

brunoerg commented at 1:07 PM on October 31, 2022:

I had the same question in my mind. I tested it manually, but the way I found to know it didn't pass de-serialization is checking the debug log: "0 block-relay-only anchors will be tried for connections.". If it had passed, it would try to connect to the 2 anchors from the file.

brunoerg commented at 6:21 PM on December 5, 2022:

Any other thoughts?
DrahtBot commented at 6:21 PM on December 5, 2022: contributor



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK MarcoFalke

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
achow101 requested review from maflcko on Apr 25, 2023
maflcko commented at 8:33 AM on April 27, 2023: member

lgtm ACK 33fdfc7986455191df8ce339261bc0561115cf7f
DrahtBot removed review request from maflcko on Apr 27, 2023
maflcko assigned fanquake on Apr 27, 2023
fanquake merged this on Apr 27, 2023
fanquake closed this on Apr 27, 2023
sidhujag referenced this in commit 42f538aa42 on Apr 28, 2023
bitcoin locked this on Apr 26, 2024

Type	Reviewers
ACK	MarcoFalke

Contributors

Labels

test: perturb anchors.dat to test error during initialization #26314

Reviews