Overview

I worked on the goals I outlined last week. While I didn’t achieve all of them, I completed satisfactory work on most. The majority of my time was devoted to reviewing potential bugs from last week, writing new tests, and identifying improvements for the existing tests—much like the previous week.

Goals accomplished

  • Investigated the potential bug in amount_msat_sub_fee(): Before Matt went on vacation, he asked me to investigate the potential bug in amount_msat_sub_fee(), which was revealed by the amount-{sat, msat} arithmetic test I wrote last week. This took much longer than expected. After a comprehensive investigation, I found that the only time the function under scrutiny—amount_msat_sub_fee()—is invoked is through the getroutes method in the plugin named askrene. I added a user-level Python test in tests/test_askrene.py to attempt to pass the error-causing parameters all the way to amount_msat_sub_fee(), but I discovered that the values are truncated to a set of safe values as soon as any parameter becomes too large.

    In summary, I have concluded that it does not appear possible for any external message to cause the breakage observed in the fuzz test. I will report these findings to Matt and decide on our next steps.

  • Investigated the potential bug in handle_peer_error_or_warning(): This was the other bug I planned to investigate this week. I looked for the callers of this function—similar to my approach for the previous bug—and discovered that it may be triggered by one of these callers, specifically one in closingd/closingd.c. I have identified the cause of the bug but am unsure how to fix it, so I will discuss it with Matt next week before deciding on an approach.

  • Reviewed the handshake tests: There are three handshake tests in the fuzz suite: fuzz-connectd-handshake-act1, fuzz-connectd-handshake-act2 and fuzz-connectd-handshake-act3. I had already examined these tests thoroughly before SoB, when I was attempting to add a stateful fuzz test for the handshaking protocol. Since they are already pretty comprehensive, I checked them off for now.

  • Reviewed bip32 and sha256 tests: These tests already cover their target files thoroughly, and I believe there is little room for hidden bugs in their scope, so I marked them as complete. I considered adding a double SHA256 check to the SHA256 test, but there is no double SHA256 method in OpenSSL for comparison, and since no bug exists in SHA256, the likelihood of finding a bug in double SHA256 is minuscule. Therefore, I decided to focus my efforts on more fruitful areas.

  • Added a test for BOLT #12 tlv_span(): While reviewing the fuzz tests for BOLT #12 methods, I noticed that tlv_span() in common/bolt12.c consumes untrusted input in the form of a tlvstream but was untested by the existing suite. I added a test for it, and although it didn’t uncover any new errors, I believe it’s a valuable addition to the fuzz suite. I plan to push this test later in the upcoming week.

  • Added a wire test for onionreply messages: While reviewing the codebase for one of the new tests I added this week, I noticed that the onionreply unmarshalling and marshalling functions—fromwire_onionreply() and towire_onionreply()—aren’t tested. I added a round-trip test for these functions, similar to the other wire fuzzers, and plan to push this change soon.

  • Added a wire test for calculate_our_funding(): While examining the plugins/ code during my investigation of the potential bug in amount_msat_sub_fee(), I noticed a test named run-funder-policy that attempted to simulate a fuzz test. I converted this into an actual fuzz test, which resulted in a failure that appears to be a genuine bug. I have conducted an initial investigation into the failure and believe it is a real issue, although some aspects remain unclear. I will write a small report on Monday and have Matt review it. We can then determine the next steps.

Next week’s goals

I plan on using the same strategy for the next week that I used this week, which is to divide my time into incrementally reviewing the existing tests and working towards writing a new one. With this being said, here are some of my immediate goals for the next week:

  • Investigate the potential bug in calculate_our_funding(): My investigation into the amount_msat_sub_fee() bug was inconclusive, but I’m optimistic about this new issue. Since I haven’t identified its root cause yet, my next step is to clearly document the situation for Matt’s review. After discussing it with him, I’ll determine my next move.

  • Reach a conclusion on the potential bug in handle_peer_error_or_warning(): I have investigated this and understand why the breakage occurs, but I’m unsure whether it’s a genuine bug or an issue with my harness. I’ve encountered the same problem in the new BOLT #2 handle_peer_in() test I’m working on, which leads me to suspect it’s a harness issue. I’ll consult Matt to reach a definitive conclusion, which I aim to do by our next meeting on Monday.

  • Review other tests: Similar to the last week, I plan to review at least five new tests next week. This will help me stay aligned with the schedule I outlined in my proposal.

  • Continue working on BOLT #2 message handlers: I completed work on the BOLT #1 message handlers this week and have already begun on the BOLT #2 handlers, starting with the open_channel handler (handle_peer_in). I will continue working on them and aim to finish the majority by the end of the week.

Challenges

As mentioned, investigating the potential bug in amount_msat_sub_fee() required far more time and effort than I anticipated. However, I’m pleased that I reached a definite conclusion in the end. I cannot say the same about the bug in handle_peer_error_or_warning(). I had hoped to fix it or reach a concrete conclusion, but I haven’t been able to, so I’ll need Matt’s help. I encounter this bug in a couple of harnesses as well, so it’s better to resolve it as early as possible.

I must admit that writing tests for Core Lightning is a chore. Earlier this week, I looked at the LDK fuzz tests in hopes of adapting something for CLN, and I noticed how much easier their testing setup is to work with. I suppose the difference between C and Rust contributes to this, but I have worked on other non-trivial C projects—Git, systemd, and others—and adding tests there was much easier. Therefore, I can’t help but attribute some of these difficulties to the project’s architecture. Alas, there’s nothing I can do except wish it were better and make do with what I have. I’ll probably have to limit what I hoped to achieve through my SoB project though.

Matt comes back this week, so our next meeting is scheduled for Monday. I’ll discuss my progress with him and hopefully sort out the potential bugs in this meeting. I’m a little disheartened because I couldn’t acheive all the goals that I had planned for the week, but I’ll give a go at it again in the upcoming week and hopefully achieve more than what I have planned.

Till next time,

Chandra.

<
Previous Post
SoB: Week 1
>
Next Post
SoB: Week 3