Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support group keys on SendPayment & AddInvoice #1423

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

GeorgeTsagk
Copy link
Member

Description

This PR takes care of the rest of the work required to create and pay invoices by using group keys.

  • Using a group_key on SendPayment means that only channels that contain assets which belong to that group are going to be considered for sending this payment.
  • Using a group_key on AddInvoice means that only channels that contain assets which belong to that group may be considered for generating an RFQ quote and encoding it in the invoice.

Based on #1382

@coveralls
Copy link

coveralls commented Mar 5, 2025

Pull Request Test Coverage Report for Build 13796748518

Details

  • 28 of 204 (13.73%) changed or added relevant lines in 7 files are covered.
  • 24371 unchanged lines in 166 files lost coverage.
  • Overall coverage decreased (-26.9%) to 27.921%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tapchannel/aux_invoice_manager.go 17 18 94.44%
rfqmsg/records.go 11 15 73.33%
taprpc/tapchannelrpc/tapchannel.pb.go 0 10 0.0%
tapchannel/aux_traffic_shaper.go 0 12 0.0%
rfq/manager.go 0 19 0.0%
rfq/order.go 0 24 0.0%
rpcserver.go 0 106 0.0%
Files with Coverage Reduction New Missed Lines %
asset/witness.go 2 91.38%
proof/util.go 2 81.63%
tapdb/migrations.go 2 74.63%
address/log.go 3 0.0%
commitment/log.go 3 0.0%
internal/pedersen/commitment.go 3 95.31%
rfq/log.go 3 0.0%
tapchannel/log.go 3 0.0%
tapdb/interfaces.go 3 59.81%
taprpc/tapdevrpc/config_active.go 3 0.0%
Totals Coverage Status
Change from base Build 13795771378: -26.9%
Covered Lines: 25383
Relevant Lines: 90911

💛 - Coveralls

@GeorgeTsagk
Copy link
Member Author

The current LiT itests are going to pass, as they will keep using a single asset ID for payments and invoices.

This LiT PR uses the new group key arguments for creating/paying invoices.

@GeorgeTsagk GeorgeTsagk force-pushed the rfq-negotiation-groupkey branch from 7c660bd to 6cf7190 Compare March 10, 2025 15:12
Base automatically changed from rfq-negotiation-groupkey to main March 11, 2025 17:31
We add a new interface to the HTLC SumAssetBalance method, which helps
check the identifier of the asset against a specifier. This allows for
checking asset inclusion in a group, which is a bit involved and not the
responsibility of the HTLC model.
We extend the interface of the rfq Policy in order to allow the
specifier checker to be involved. This extends certain checks, and
allows us to use asset specifiers that only have a group key.
We add the specifier checker interface to the AuxInvoiceManager too, as
it is needed to validate incoming HTLCs which may use asset IDs that
belong to a group, while the RFQ is based on a group key.
Adds some coverage to the invoice manager unit tests, which involve an
RFQ quote over a group key, plus an HTLC with multiple asset balances,
which may belong or not to the group.
We may be performing a group lookup on an asset that doesn't belong to a
group. Instead of returning the error that originates from the ErrNoRows
of sql we instead return a nil result, signalling that no group was
found and no error occurred.
In some cases the sender of an asset HTLC may just encode the hash of
the group key as the assetID of the balance in the custom records. This
is done as a way to signal that any asset ID that belongs to this group
may be picked to carry out the payment. This commit makes the specifier
matcher aware of that case.
As mentioned in the previous commit, we occasionally want to just encode
the hash of the group key as the asset ID of the balance that is encoded
in the custom record. We make the ProduceHtlcExtraData hook aware of
that case, which is triggered when the quote is made upon a group key
and not a specific asset ID.
We have taken care of the groupkey RFQ negotiation in previous commits.
All we need now to support sending a payment over a group of assets, is
to propagate the user specifier groupkey to the corresponding fields.
In this commit we take the user defined group key and allow the asset
specifier to be created over it. All the calls that accept it as an
argument are already groupkey aware.
@GeorgeTsagk GeorgeTsagk force-pushed the taprpc-groupkey-support branch from 9c9de09 to b53b3a2 Compare March 11, 2025 19:40
@GeorgeTsagk
Copy link
Member Author

Rebased on main after merging base branch #1382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

2 participants