Skip to content

Conversation

@marcofleon
Copy link
Contributor

@marcofleon marcofleon commented Jun 5, 2024

Addresses #28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as GetTime is called at points in i2p. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in i2p every time, so the fuzzer can eventually make progress through the target code.

Re-working this harness also led me and dergoegge to resolve a couple of issues in FuzzedSock, which allows for full coverage of the i2p code. Those changes can be seen in #30211.

The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.

I2P dict
"HELLO VERSION"
"HELLO REPLY RESULT=OK VERSION="
"HELLO REPLY RESULT=NOVERSION"
"HELLO REPLY RESULT=I2P_ERROR"
"SESSION CREATE"
"SESSION STATUS RESULT=OK DESTINATION="
"SESSION STATUS RESULT=DUPLICATED_ID"
"SESSION STATUS RESULT=DUPLICATED_DEST"
"SESSION STATUS RESULT=INVALID_ID"
"SESSION STATUS RESULT=INVALID_KEY"
"SESSION STATUS RESULT=I2P_ERROR MESSAGE="
"SESSION ADD"
"SESSION REMOVE"
"STREAM CONNECT"
"STREAM STATUS RESULT=OK"
"STREAM STATUS RESULT=INVALID_ID"
"STREAM STATUS RESULT=INVALID_KEY"
"STREAM STATUS RESULT=CANT_REACH_PEER"
"STREAM STATUS RESULT=I2P_ERROR MESSAGE="
"STREAM ACCEPT"
"STREAM FORWARD"
"DATAGRAM SEND"
"RAW SEND"
"DEST GENERATE"
"DEST REPLY PUB= PRIV="
"DEST REPLY RESULT=I2P_ERROR"
"NAMING LOOKUP"
"NAMING REPLY RESULT=OK NAME= VALUE="
"DATAGRAM RECEIVED DESTINATION= SIZE="
"RAW RECEIVED SIZE="
"NAMING REPLY RESULT=INVALID_KEY NAME="
"NAMING REPLY RESULT=KEY_NOT_FOUND NAME="
"MIN"
"MAX"
"STYLE"
"ID"
"SILENT"
"DESTINATION"
"NAME"
"SIGNATURE_TYPE"
"CRYPTO_TYPE"
"SIZE"
"HOST"
"PORT"
"FROM_PORT"
"TRANSIENT"
"STREAM"
"DATAGRAM"
"RAW"
"MASTER"
"true"
"false"

I'll add this dict to qa-assets later on.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 5, 2024

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK dergoegge, brunoerg, vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Jun 5, 2024
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 5, 2024

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/25842896976

@DrahtBot DrahtBot removed the CI failed label Jun 5, 2024
@marcofleon marcofleon force-pushed the 2024/05/add-i2p-test branch from b9a754f to 193c748 Compare June 6, 2024 20:08
Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 193c748

Thanks for picking this up!

I fuzzed the i2p harrness for a few hundred CPU hours and even without the dictionary it reaches almost everything. The dictionary will still help though, so adding it to qa-assets later would be good.

My only note: I think it would be worthwhile to point out in the PR description what changes you have made to the original harness and why they were necessary.

@marcofleon
Copy link
Contributor Author

My only note: I think it would be worthwhile to point out in the PR description what changes you have made to the original harness and why they were necessary.

Done. Thanks for all the help @dergoegge.

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 193c748

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 193c748

@fanquake fanquake merged commit d0cb516 into bitcoin:master Jun 12, 2024
@marcofleon marcofleon deleted the 2024/05/add-i2p-test branch November 19, 2024 18:44
PastaPastaPasta pushed a commit to DashCoreAutoGuix/dash that referenced this pull request May 16, 2025
193c748 fuzz: add I2P harness (marcofleon)

Pull request description:

  Addresses bitcoin#28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as `GetTime` is called at points in `i2p`. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in `i2p` every time, so the fuzzer can eventually make progress through the target code.

  Re-working this harness also led me and dergoegge to resolve a couple of issues in `FuzzedSock`, which allows for full coverage of the `i2p` code. Those changes can be seen in bitcoin#30211.

  The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.

  <details>
  <summary>I2P dict</summary>

  ```
  "HELLO VERSION"
  "HELLO REPLY RESULT=OK VERSION="
  "HELLO REPLY RESULT=NOVERSION"
  "HELLO REPLY RESULT=I2P_ERROR"
  "SESSION CREATE"
  "SESSION STATUS RESULT=OK DESTINATION="
  "SESSION STATUS RESULT=DUPLICATED_ID"
  "SESSION STATUS RESULT=DUPLICATED_DEST"
  "SESSION STATUS RESULT=INVALID_ID"
  "SESSION STATUS RESULT=INVALID_KEY"
  "SESSION STATUS RESULT=I2P_ERROR MESSAGE="
  "SESSION ADD"
  "SESSION REMOVE"
  "STREAM CONNECT"
  "STREAM STATUS RESULT=OK"
  "STREAM STATUS RESULT=INVALID_ID"
  "STREAM STATUS RESULT=INVALID_KEY"
  "STREAM STATUS RESULT=CANT_REACH_PEER"
  "STREAM STATUS RESULT=I2P_ERROR MESSAGE="
  "STREAM ACCEPT"
  "STREAM FORWARD"
  "DATAGRAM SEND"
  "RAW SEND"
  "DEST GENERATE"
  "DEST REPLY PUB= PRIV="
  "DEST REPLY RESULT=I2P_ERROR"
  "NAMING LOOKUP"
  "NAMING REPLY RESULT=OK NAME= VALUE="
  "DATAGRAM RECEIVED DESTINATION= SIZE="
  "RAW RECEIVED SIZE="
  "NAMING REPLY RESULT=INVALID_KEY NAME="
  "NAMING REPLY RESULT=KEY_NOT_FOUND NAME="
  "MIN"
  "MAX"
  "STYLE"
  "ID"
  "SILENT"
  "DESTINATION"
  "NAME"
  "SIGNATURE_TYPE"
  "CRYPTO_TYPE"
  "SIZE"
  "HOST"
  "PORT"
  "FROM_PORT"
  "TRANSIENT"
  "STREAM"
  "DATAGRAM"
  "RAW"
  "MASTER"
  "true"
  "false"
  ```

  </details>

  I'll add this dict to qa-assets later on.

ACKs for top commit:
  dergoegge:
    tACK 193c748
  brunoerg:
    ACK 193c748
  vasild:
    ACK 193c748

Tree-SHA512: 09ae4b3fa0738aa6f159f4d920493bdbce786b489bc8148e7a135a881e9dba93d727b40f5400c9510e218dd2cfdccc7ce2d3ac9450654fb29c78aac59af92ec3
knst pushed a commit to knst/dash that referenced this pull request Oct 22, 2025
193c748 fuzz: add I2P harness (marcofleon)

Pull request description:

  Addresses bitcoin#28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as `GetTime` is called at points in `i2p`. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in `i2p` every time, so the fuzzer can eventually make progress through the target code.

  Re-working this harness also led me and dergoegge to resolve a couple of issues in `FuzzedSock`, which allows for full coverage of the `i2p` code. Those changes can be seen in bitcoin#30211.

  The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.

  <details>
  <summary>I2P dict</summary>

  ```
  "HELLO VERSION"
  "HELLO REPLY RESULT=OK VERSION="
  "HELLO REPLY RESULT=NOVERSION"
  "HELLO REPLY RESULT=I2P_ERROR"
  "SESSION CREATE"
  "SESSION STATUS RESULT=OK DESTINATION="
  "SESSION STATUS RESULT=DUPLICATED_ID"
  "SESSION STATUS RESULT=DUPLICATED_DEST"
  "SESSION STATUS RESULT=INVALID_ID"
  "SESSION STATUS RESULT=INVALID_KEY"
  "SESSION STATUS RESULT=I2P_ERROR MESSAGE="
  "SESSION ADD"
  "SESSION REMOVE"
  "STREAM CONNECT"
  "STREAM STATUS RESULT=OK"
  "STREAM STATUS RESULT=INVALID_ID"
  "STREAM STATUS RESULT=INVALID_KEY"
  "STREAM STATUS RESULT=CANT_REACH_PEER"
  "STREAM STATUS RESULT=I2P_ERROR MESSAGE="
  "STREAM ACCEPT"
  "STREAM FORWARD"
  "DATAGRAM SEND"
  "RAW SEND"
  "DEST GENERATE"
  "DEST REPLY PUB= PRIV="
  "DEST REPLY RESULT=I2P_ERROR"
  "NAMING LOOKUP"
  "NAMING REPLY RESULT=OK NAME= VALUE="
  "DATAGRAM RECEIVED DESTINATION= SIZE="
  "RAW RECEIVED SIZE="
  "NAMING REPLY RESULT=INVALID_KEY NAME="
  "NAMING REPLY RESULT=KEY_NOT_FOUND NAME="
  "MIN"
  "MAX"
  "STYLE"
  "ID"
  "SILENT"
  "DESTINATION"
  "NAME"
  "SIGNATURE_TYPE"
  "CRYPTO_TYPE"
  "SIZE"
  "HOST"
  "PORT"
  "FROM_PORT"
  "TRANSIENT"
  "STREAM"
  "DATAGRAM"
  "RAW"
  "MASTER"
  "true"
  "false"
  ```

  </details>

  I'll add this dict to qa-assets later on.

ACKs for top commit:
  dergoegge:
    tACK 193c748
  brunoerg:
    ACK 193c748
  vasild:
    ACK 193c748

Tree-SHA512: 09ae4b3fa0738aa6f159f4d920493bdbce786b489bc8148e7a135a881e9dba93d727b40f5400c9510e218dd2cfdccc7ce2d3ac9450654fb29c78aac59af92ec3
@bitcoin bitcoin locked and limited conversation to collaborators Nov 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants