Skip to content

Fix BOLT 11 spec compliance: skip unknown fallback address versions#81

Merged
nGoline merged 1 commit into
nGoline:mainfrom
jklein24:main
Feb 5, 2026
Merged

Fix BOLT 11 spec compliance: skip unknown fallback address versions#81
nGoline merged 1 commit into
nGoline:mainfrom
jklein24:main

Conversation

@jklein24

@jklein24 jklein24 commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Per BOLT 11 spec, readers "MUST skip over f fields that use an unknown version" and "versions 19-31 will be ignored by readers."

https://github.com/lightning/bolts/blob/master/11-payment-encoding.md#rationale-2

The context is that Spark embeds a fallback spark address with version 31 which should be ignored by lightning implementations as per the BOLT11 spec, but instead nlightning is throwing an error when parsing a spark lightning invoice.

Previously, FallbackAddressTaggedField.FromBitReader threw an exception for unknown address versions. Now it returns null, which the existing TaggedFieldList already handles by skipping the field.

Changes:

  • Return null instead of throwing for unknown versions (1-16, 19-31)
  • Update return type to FallbackAddressTaggedField?
  • Add tests for unknown version handling
  • Add Assert.NotNull to existing tests for nullable return type

@nGoline nGoline added the bug Something isn't working label Feb 4, 2026
@nGoline

nGoline commented Feb 4, 2026

Copy link
Copy Markdown
Owner

Hi @jklein24, first of all thank you for the fix.

I just need you to sign the commit so I can proceed with the merge.

Per BOLT 11 spec, readers "MUST skip over `f` fields that use an
unknown `version`" and "versions 19-31 will be ignored by readers."

Previously, FallbackAddressTaggedField.FromBitReader threw an exception
for unknown address versions. Now it returns null, which the existing
TaggedFieldList already handles by skipping the field.

Changes:
- Return null instead of throwing for unknown versions (1-16, 19-31)
- Update return type to FallbackAddressTaggedField?
- Add tests for unknown version handling
- Add Assert.NotNull to existing tests for nullable return type

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jklein24

jklein24 commented Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jklein24, first of all thank you for the fix.

I just need you to sign the commit so I can proceed with the merge.

Done!

@nGoline nGoline left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Changes are sound, tests are written, LGTM!

@github-actions

github-actions Bot commented Feb 5, 2026

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
NLightning.Application 30% 22% 258
NLightning.Bolt11 89% 84% 506
NLightning.Bolt11.Blazor 0% 0% 436
NLightning.Domain 73% 50% 1571
NLightning.Infrastructure 56% 47% 1004
NLightning.Infrastructure.Bitcoin 56% 50% 467
NLightning.Infrastructure.Persistence 0% 0% 156
NLightning.Infrastructure.Persistence.Postgres 0% 100% 7
NLightning.Infrastructure.Persistence.Sqlite 0% 100% 4
NLightning.Infrastructure.Persistence.SqlServer 0% 100% 7
NLightning.Infrastructure.Repositories 0% 0% 474
NLightning.Infrastructure.Serialization 67% 51% 861
NLightning.Node 10% 12% 215
NLightning.Tests.Utils 84% 50% 19
Summary 42% (6657 / 15907) 43% (1760 / 4119) 5985

@nGoline nGoline merged commit 4706732 into nGoline:main Feb 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants