fuzz-tests: Add a test for `calculate_our_funding()` #8312

Chand-ra · 2025-06-02T07:11:38Z

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

The changelog has been updated in the relevant commit(s) according to the guidelines.
Tests have been added or modified to reflect the changes.
Documentation has been reviewed and updated as needed.
Related issues have been listed and linked, including any that this PR closes.

Chand-ra · 2025-06-02T07:13:50Z

Hey @morehouse ,

this test fails with the error:

our_funds 0 < per_channel_min 18446744073709551615

but the function under test, plugins/funder_policy.c::calculate_our_funding(), has a check that's supposed to guard against this error:

if (amount_sat_less(*our_funding, policy->per_channel_min)) {
	*our_funding = AMOUNT_SAT(0);
	return error;
...
...
...
}

What could be the cause? I tried inspecting the breakage using gdb but the values upon inspection seem to vary from the error output. I've added the breaking input as the corpus, I'd appreciate your thoughts on this.

morehouse · 2025-06-07T18:48:14Z

When it fails, is our_funds always 0?

The check is skipped when the returned amount is 0.

lightning/plugins/funder_policy.c

Lines 297 to 299 in ad2dc97

    
           /* Don't return an 'error' if we're already at 0 */ 
        
           if (amount_sat_is_zero(*our_funding)) 
        
           	return NULL;

Chand-ra · 2025-06-11T05:32:13Z

This test is now ready to review.

morehouse · 2025-06-12T16:52:27Z

tests/fuzz/fuzz-funder-policy.c

+{ fprintf(stderr, "json_add_string called!\n"); abort(); }
+/* AUTOGENERATED MOCKS END */
+
+#define MAX_BTC ((u32)WALLY_SATOSHI_PER_BTC * WALLY_BTC_MAX)


I'm pretty sure this multiplication overflows u32.

I copied this over from tests/fuzz/fuzz-close_tx.c:

57 max = AMOUNT_SAT((u32)WALLY_SATOSHI_PER_BTC * WALLY_BTC_MAX);

Should I fix it there as well? Possibly by making this small change:

(u64)WALLY_SATOSHI_PER_BTC

Yes, please create a separate PR for that fix and then run the fuzz target to make sure it still works.

morehouse · 2025-06-12T17:17:40Z

tests/fuzz/fuzz-funder-policy.c

+static struct amount_sat fromwire_amount_sat_bounded(const u8 **cursor, size_t *max)
+{
+	struct amount_sat amt = fromwire_amount_sat(cursor, max);
+	 amt.satoshis %= (MAX_BTC + 1);


Nit: whitespace

Suggested change

amt.satoshis %= (MAX_BTC + 1);

amt.satoshis %= (MAX_BTC + 1);

morehouse · 2025-06-12T17:22:04Z

tests/fuzz/fuzz-funder-policy.c

+
+void run(const u8 *data, size_t size)
+{
+	if (size < 85)


Since data is only ever read using fromwire* functions, we probably don't need to check size at all. If we run out of data, fromwire should automatically return 0 values.

morehouse · 2025-06-12T17:48:57Z

tests/fuzz/fuzz-funder-policy.c

+	struct amount_sat channel_size;
+	struct amount_sat lease_request;
+	struct funder_policy policy;
+	struct amount_sat exp_our_funds;


exp_our_funds is unused.

morehouse · 2025-06-12T18:00:07Z

tests/fuzz/fuzz-funder-policy.c

+	struct amount_sat their_funds;
+	struct amount_sat available_funds;
+	struct amount_sat *our_last_funds;
+	struct amount_sat channel_size;


This is the max channel size allowed.

Suggested change

struct amount_sat channel_size;

struct amount_sat max_channel_size;

Changelog-None: `calculate_our_funding()` in `plugins/funder_policy.c` is responsible for calculating our funding policy. Add a test for it.

Add a minimal input set as a seed corpus for the newly introduced test. This leads to discovery of interesting code paths faster.

Chand-ra · 2025-06-19T10:17:48Z

Hey @morehouse, I have incorporated the above suggestions. Regarding the policy->mod bounds that we discussed yesterday, I found this article that documents them and I’ve integrated that information into the test as well. The updated test no longer breaks.

morehouse

This target leaks memory:

==280594== ERROR: libFuzzer: out-of-memory (used: 2067Mb; limit: 2048Mb)        
   To change the out-of-memory limit use -rss_limit_mb=<N>                      
                                                                                
Live Heap Allocations: 1131892148 bytes in 20471300 chunks; quarantined: 2801018
 bytes in 29456 chunks; 1788983 other chunks; total chunks: 22289739; showing to
p 95% (at most 8 unique contexts)

I think clean_tmpctx should fix it.

morehouse · 2025-06-20T15:39:18Z

tests/fuzz/fuzz-funder-policy.c

+	struct amount_sat our_last_funds_val;
+	if (flag)
+	{
+		our_last_funds_val = fromwire_amount_sat_bounded(&data, &size);
+		tcase.our_last_funds = &our_last_funds_val;
+	}


Nit: we can reduce the scope of our_last_funds_val

Suggested change

struct amount_sat our_last_funds_val;

if (flag)

{

our_last_funds_val = fromwire_amount_sat_bounded(&data, &size);

tcase.our_last_funds = &our_last_funds_val;

}

if (flag)

{

struct amount_sat our_last_funds_val = fromwire_amount_sat_bounded(&data, &size);

tcase.our_last_funds = &our_last_funds_val;

}

morehouse · 2025-06-20T16:03:11Z

tests/fuzz/fuzz-funder-policy.c

+	tcase.policy.fuzz_factor = fromwire_u8(&data, &size) % 101;
+	tcase.policy.reserve_tank = fromwire_amount_sat_bounded(&data, &size);
+	tcase.policy.fund_probability = fromwire_u8(&data, &size) % 101;
+	tcase.policy.leases_only = fromwire_u8(&data, &size) & 1;


Nit: we could improve readability by creating a helper function to populate tcase for us.

morehouse · 2025-06-20T16:04:32Z

tests/fuzz/fuzz-funder-policy.c

+
+	/* Validate invariants */
+	if (err)
+		tal_free(err);


It seems this isn't freeing all the memory that calculate_our_funding allocates. Let's replace this with clean_tmpctx on every iteration.

Chand-ra force-pushed the funder-policy branch 2 times, most recently from 01b2f27 to 776eea0 Compare June 11, 2025 05:31

Chand-ra marked this pull request as ready for review June 11, 2025 05:31

morehouse reviewed Jun 12, 2025

View reviewed changes

Chandra Pratap added 2 commits June 19, 2025 09:58

fuzz-tests: Add a test for calculate_our_funding()

d097386

Changelog-None: `calculate_our_funding()` in `plugins/funder_policy.c` is responsible for calculating our funding policy. Add a test for it.

fuzz-tests: Add a seed corpus for the new test

997ac30

Add a minimal input set as a seed corpus for the newly introduced test. This leads to discovery of interesting code paths faster.

Chand-ra force-pushed the funder-policy branch from 776eea0 to 997ac30 Compare June 19, 2025 10:17

morehouse suggested changes Jun 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fuzz-tests: Add a test for `calculate_our_funding()` #8312

fuzz-tests: Add a test for `calculate_our_funding()` #8312

Chand-ra commented Jun 2, 2025 •

edited

Loading

Uh oh!

Chand-ra commented Jun 2, 2025

Uh oh!

morehouse commented Jun 7, 2025

Uh oh!

Chand-ra commented Jun 11, 2025

Uh oh!

morehouse Jun 12, 2025

Uh oh!

Chand-ra Jun 13, 2025 •

edited

Loading

Uh oh!

morehouse Jun 17, 2025

Uh oh!

morehouse Jun 12, 2025

Uh oh!

morehouse Jun 12, 2025

Uh oh!

morehouse Jun 12, 2025

Uh oh!

morehouse Jun 12, 2025

Uh oh!

Chand-ra commented Jun 19, 2025

Uh oh!

morehouse left a comment

Uh oh!

morehouse Jun 20, 2025

Uh oh!

morehouse Jun 20, 2025

Uh oh!

morehouse Jun 20, 2025

Uh oh!

Uh oh!

	amt.satoshis %= (MAX_BTC + 1);
	amt.satoshis %= (MAX_BTC + 1);

	struct amount_sat channel_size;
	struct amount_sat max_channel_size;

fuzz-tests: Add a test for calculate_our_funding() #8312

Are you sure you want to change the base?

fuzz-tests: Add a test for calculate_our_funding() #8312

Conversation

Chand-ra commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Chand-ra commented Jun 2, 2025

Uh oh!

morehouse commented Jun 7, 2025

Uh oh!

Chand-ra commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chand-ra Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chand-ra commented Jun 19, 2025

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fuzz-tests: Add a test for `calculate_our_funding()` #8312

fuzz-tests: Add a test for `calculate_our_funding()` #8312

Chand-ra commented Jun 2, 2025 •

edited

Loading

Chand-ra Jun 13, 2025 •

edited

Loading