Fixes stuff inside bolt12 decode #324

Harshit933 · 2024-12-14T07:59:52Z

Decoding bolt12 now:

[harshit:~] lampo-cli --network regtest decode --invoice_str lno1qgsyxjtl6luzd9t3pr62xr7eemp6awnejusgf6gw45q75vcfqqqqqqqsespexwyy4tcadvgg89l9aljus6709kx235hhqrk6n8dey98uyuftzdqzrtkahuum7m56dxlnx8r6tffy54004l7kvs7pylmxx7xs4n54986qyqeeuhhunayntt50snmdkq4t7fzsgghpl69v9csgparek8kv7dlp5uqr8ymp5s4z9upmwr2s8xu020d45t5phqc8nljrq8gzsjmurzevawjz6j6rc95xwfvnhgfx6v4c3jha7jwynecrz3y092nn25ek4yl7xp9yu9ry9zqagt0ktn4wwvqg52v9ss9ls22sqyqqestzp2l6decpn87pq96udsvx
{
  "issuer": "No issuer id provided",
  "offer_id": "[52, 70, 8, 105, 84, 158, 55, 116, 140, 234, 171, 220, 255, 98, 132, 169, 130, 102, 193, 128, 82, 171, 42, 126, 158, 181, 161, 175, 10, 94, 91, 125]",
  "offer_chains": [
    "43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea330900000000"
  ],
  "description": "Description not provided",
  "offer_paths": [
    {
      "blinded_hops": [
        "0339e5efc9f4935ae8f84f6db02abf2450422e1fe8ac2e2080f479b1eccf37e1a7",
        "031448f2aa7355336a93fe304a4e14642881d42df65ceae73008a2985840bf8295"
      ],
      "blinding_points": "021aeddbf39bf6e9a69bf331c7a5a524a55efaffd6643c127f66378d0ace9529f4"
    }
  ],
  "network": "testnet"
}

If the bolt11/bolt12 is invalid:

[harshit:~] lampo-cli --network regtest decode --invoice_str lno1qgsyxjtl6luzd9t3pr62xr7eemp6awnejusgf6gw45q75vcfqqqqqqqsespexwyy4tcadvgg89l9aljus6709kx235hhqrk6n8dey98uyuftzdqzrtkahuum7m56dxlnx8r6tffy54004l7kvs7pylmxx7xs4n54986qyqeeuhhunayntt50snmdkq4t7fzsgghpl69v9csgparek8kv7dlp5uqr8ymp5s4z9upmwr2s8xu020d45t5phqc8nljrq8gzsjmurzevawjz6j6rc95xwfvnhgfx6v4c3jha7jwynecrz3y092nn25ek4yl7xp9yu9ry9zqagt0ktn4wwvqg52v9ss9ls22sqyqqestzp2l6decpn87pq96uds
{
  "code": -1,
  "message": "Not able to decode invoice",
  "data": null
}

UPDATE:

Running lampo-cli --network regtest decode --invoice_str lno1qgsyxjtl6luzd9t3pr62xr7eemp6awnejusgf6gw45q75vcfqqqqqqqsespexwyy4tcadvgg89l9aljus6709kx235hhqrk6n8dey98uyuftzdqzrtkahuum7m56dxlnx8r6tffy54004l7kvs7pylmxx7xs4n54986qyqeeuhhunayntt50snmdkq4t7fzsgghpl69v9csgparek8kv7dlp5uqr8ymp5s4z9upmwr2s8xu020d45t5phqc8nljrq8gzsjmurzevawjz6j6rc95xwfvnhgfx6v4c3jha7jwynecrz3y092nn25ek4yl7xp9yu9ry9zqagt0ktn4wwvqg52v9ss9ls22sqyqqestzp2l6decpn87pq96udsvx

Gives the following output:

{
  "issuer_id": null,
  "offer_id": "34460869549e37748ceaabdcff6284a98266c18052ab2a7e9eb5a1af0a5e5b7d",
  "offer_chains": [
    "43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea330900000000"
  ],
  "description": null,
  "offer_paths": [
    {
      "blinded_hops": [
        "0339e5efc9f4935ae8f84f6db02abf2450422e1fe8ac2e2080f479b1eccf37e1a7",
        "031448f2aa7355336a93fe304a4e14642881d42df65ceae73008a2985840bf8295"
      ],
      "blinding_points": "021aeddbf39bf6e9a69bf331c7a5a524a55efaffd6643c127f66378d0ace9529f4"
    }
  ],
  "network": "testnet"
}

See commits for more info.
Fixes #270 #318

lampod/src/jsonrpc/offchain.rs

vincenzopalazzo · 2024-12-14T10:26:17Z

Plus offer_id is a vector of bytes, please convert it to hex::(offer_id) there is no need to print a vector of byte to the user

vincenzopalazzo · 2025-01-16T08:22:46Z

@Harshit933 can you post another time the lampo-cli --network regtest decode --invoice_str lno1qgsyxjtl6luzd9t3pr62xr7eemp6awnejusgf6gw45q75vcfqqqqqqqsespexwyy4tcadvgg89l9aljus6709kx235hhqrk6n8dey98uyuftzdqzrtkahuum7m56dxlnx8r6tffy54004l7kvs7pylmxx7xs4n54986qyqeeuhhunayntt50snmdkq4t7fzsgghpl69v9csgparek8kv7dlp5uqr8ymp5s4z9upmwr2s8xu020d45t5phqc8nljrq8gzsjmurzevawjz6j6rc95xwfvnhgfx6v4c3jha7jwynecrz3y092nn25ek4yl7xp9yu9ry9zqagt0ktn4wwvqg52v9ss9ls22sqyqqestzp2l6decpn87pq96udsvx

And/or probably write a test to make sure that what we should have as a hex string it is not a vector of bytes?

otherwise LGMT Thanks :)

Harshit933 · 2025-01-23T23:05:42Z

@Harshit933 can you post another time the lampo-cli --network regtest decode --invoice_str lno1qgsyxjtl6luzd9t3pr62xr7eemp6awnejusgf6gw45q75vcfqqqqqqqsespexwyy4tcadvgg89l9aljus6709kx235hhqrk6n8dey98uyuftzdqzrtkahuum7m56dxlnx8r6tffy54004l7kvs7pylmxx7xs4n54986qyqeeuhhunayntt50snmdkq4t7fzsgghpl69v9csgparek8kv7dlp5uqr8ymp5s4z9upmwr2s8xu020d45t5phqc8nljrq8gzsjmurzevawjz6j6rc95xwfvnhgfx6v4c3jha7jwynecrz3y092nn25ek4yl7xp9yu9ry9zqagt0ktn4wwvqg52v9ss9ls22sqyqqestzp2l6decpn87pq96udsvx

Updated the description also added a test in 593fe3e

vincenzopalazzo

Ok I did another review, thanks for doing it.

I think the git history need some kind of rework (squash the commit with the one that are no longer true) and then we are ready to go :)

Thanks for doing it

lampod/src/jsonrpc/offchain.rs

lampo-common/src/model/invoice.rs

lampo-testing/src/lib.rs

tests/tests/src/lampo_tests.rs

This commit adds another struct namely `Bolt12Invoice` to structure all the information regarding it. Also renames the `InvoiceInfo` to `Bolt11InvoiceInfo`, mainly for holding bolt11 info.

This commits renames the previous testing server command for decoding offers ("decode_invoice") to `decode` and also changes all its occurings in the testing code.

vincenzopalazzo

Concept ACK 6a02c69

Small comments on the code still, but we are good to go! thanks for working on it

lampo-common/src/model/invoice.rs

lampod/src/jsonrpc/offchain.rs

lampo-testing/src/lib.rs

This commit adds an integration test to check if the `offer_id` is a hex string or not.

vincenzopalazzo

LGTM

vincenzopalazzo · 2025-02-10T11:45:14Z

lampo-common/src/model/invoice.rs

            let network = offer
                .chains()
                .first()
-                .map(|hash| Network::from_chain_hash(*hash))
-                .unwrap_or(Some(Network::Bitcoin));
+                .and_then(|hash| Network::from_chain_hash(*hash))
+                .or(Some(Network::Bitcoin));


Good also for a folloup PR, but I would like to leave this Optional and not try to force the type here!

If there is any chain, will remain None

vincenzopalazzo · 2025-02-10T11:45:46Z

tests/tests/src/lampo_tests.rs

+    // Checks if all the character inside the offer_id is hex.
+    let res = decode.offer_id.chars().all(|c| c.is_ascii_hexdigit());
+    assert!(res);


we can remove this code, I do not understand it, so better remove it (also in a follup PR)

This removes the validation check in hex as discussed in issues vincenzopalazzo#351 and vincenzopalazzo#324.

This removes the validation check in hex as discussed in issues #351 and #324.

Harshit933 requested a review from vincenzopalazzo as a code owner December 14, 2024 07:59

vincenzopalazzo requested changes Dec 14, 2024

View reviewed changes

lampod/src/jsonrpc/offchain.rs Outdated Show resolved Hide resolved

lampod/src/jsonrpc/offchain.rs Outdated Show resolved Hide resolved

vincenzopalazzo added this to the v24.12 milestone Jan 16, 2025

vincenzopalazzo added the P-hight Hiight Priotity issue label Jan 16, 2025

vincenzopalazzo assigned vincenzopalazzo and Harshit933 Jan 16, 2025

vincenzopalazzo requested changes Jan 26, 2025

View reviewed changes

Harshit933 force-pushed the feature/fix-bolt12 branch 2 times, most recently from d25a806 to 5e19836 Compare January 27, 2025 02:02

Harshit933 added 2 commits January 27, 2025 07:37

Add another struct for Bolt12Invoice

68d57c9

This commit adds another struct namely `Bolt12Invoice` to structure all the information regarding it. Also renames the `InvoiceInfo` to `Bolt11InvoiceInfo`, mainly for holding bolt11 info.

Rename decode_invoice to decode

ea1fcf3

This commits renames the previous testing server command for decoding offers ("decode_invoice") to `decode` and also changes all its occurings in the testing code.

Harshit933 force-pushed the feature/fix-bolt12 branch 2 times, most recently from 6a3a72c to 6a02c69 Compare January 27, 2025 02:14

vincenzopalazzo reviewed Jan 28, 2025

View reviewed changes

lampo-common/src/model/invoice.rs Outdated Show resolved Hide resolved

lampo-common/src/model/invoice.rs Outdated Show resolved Hide resolved

lampod/src/jsonrpc/offchain.rs Outdated Show resolved Hide resolved

lampo-testing/src/lib.rs Show resolved Hide resolved

Add an integration test for bolt12 offer_id

035af1b

This commit adds an integration test to check if the `offer_id` is a hex string or not.

Harshit933 force-pushed the feature/fix-bolt12 branch from 6a02c69 to 035af1b Compare February 8, 2025 06:15

vincenzopalazzo approved these changes Feb 10, 2025

View reviewed changes

vincenzopalazzo merged commit 74010e7 into vincenzopalazzo:main Feb 10, 2025
3 of 5 checks passed

This was referenced Feb 10, 2025

Clean up the test code #351

Closed

Fo not foce the Option value to a specific value #352

Closed

khadar1020 mentioned this pull request Feb 10, 2025

Remove hex validation check in test #353

Merged

khadar1020 pushed a commit to khadar1020/lampo.rs that referenced this pull request Feb 11, 2025

test: Remove hex validation check

2c6788c

This removes the validation check in hex as discussed in issues vincenzopalazzo#351 and vincenzopalazzo#324.

Psycho-Pirate mentioned this pull request Feb 17, 2025

minor fixes made #359

Merged

vincenzopalazzo pushed a commit that referenced this pull request Feb 17, 2025

test: Remove hex validation check

e39bac3

This removes the validation check in hex as discussed in issues #351 and #324.

vincenzopalazzo mentioned this pull request Mar 8, 2025

Lampo panics on invalid bolt12 #318

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes stuff inside bolt12 decode #324

Fixes stuff inside bolt12 decode #324

Harshit933 commented Dec 14, 2024 •

edited

Loading

vincenzopalazzo commented Dec 14, 2024

vincenzopalazzo commented Jan 16, 2025

Harshit933 commented Jan 23, 2025

vincenzopalazzo left a comment

vincenzopalazzo left a comment

vincenzopalazzo left a comment

vincenzopalazzo Feb 10, 2025

vincenzopalazzo Feb 10, 2025

Fixes stuff inside bolt12 decode #324

Fixes stuff inside bolt12 decode #324

Conversation

Harshit933 commented Dec 14, 2024 • edited Loading

vincenzopalazzo commented Dec 14, 2024

vincenzopalazzo commented Jan 16, 2025

Harshit933 commented Jan 23, 2025

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo Feb 10, 2025

Choose a reason for hiding this comment

vincenzopalazzo Feb 10, 2025

Choose a reason for hiding this comment

Harshit933 commented Dec 14, 2024 •

edited

Loading