Allow setting sats/msats to `taprpc.AddInvoice`

[GeorgeTsagk]

Description

This PR allows for the user to set the invoice value via the Value/ValueMsat fields.

Previously we'd take the asset units and the agreed upon quote, then we'd convert that to an msat amount. Now the value of the invoice can be directly set via the previously ignored fields, while also still acquiring a sufficient quote and returning that to the user.

Closes #1440

[coveralls]

Pull Request Test Coverage Report for Build 14488405809

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 14 of 147 (9.52%) changed or added relevant lines in 3 files are covered.
• 48 unchanged lines in 10 files lost coverage.
• Overall coverage decreased (-0.05%) to 28.327%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rfq/manager.go	0	21	0.0%
rpcserver.go	0	112	0.0%
<!--	Total:	14	147

Files with Coverage Reduction	New Missed Lines	%
rpcserver.go	1	0.0%
asset/group_key.go	2	57.89%
tapchannel/aux_leaf_signer.go	2	43.08%
tappsbt/create.go	2	26.74%
asset/asset.go	3	48.1%
address/address.go	6	67.47%
asset/mock.go	6	63.56%
commitment/tap.go	6	71.36%
tapgarden/caretaker.go	6	68.63%
address/mock.go	14	89.54%
<!--	Total:	48

Totals
Change from base Build 14474635473:	-0.05%
Covered Lines:	25922
Relevant Lines:	91511

---

💛 - Coveralls

[ZZiigguurraatt]

Getting the following error when trying to build litd with this PR

$ make

===== skipping part about the web UI build ==========

 Building lightning-terminal.
go build -v -tags="litd autopilotrpc signrpc walletrpc chainrpc invoicesrpc watchtowerrpc neutrinorpc peersrpc" -ldflags " -X github.com/lightningnetwork/lnd/build.Commit=lightning-terminal-v0.14.1-alpha-82-gbad957bedc7f37950de33f5f15a3f00ed29e8f8d-dirty -X github.com/lightningnetwork/lnd/build.CommitHash=bad957bedc7f37950de33f5f15a3f00ed29e8f8d -X github.com/lightningnetwork/lnd/build.GoVersion= -X github.com/lightningnetwork/lnd/build.RawTags=litd,autopilotrpc,signrpc,walletrpc,chainrpc,invoicesrpc,watchtowerrpc,neutrinorpc,peersrpc -X github.com/lightninglabs/lightning-terminal.appFilesPrefix= -X github.com/lightninglabs/lightning-terminal.Commit=v0.14.1-alpha-82-gbad957bedc7f37950de33f5f15a3f00ed29e8f8d-dirty -X github.com/lightninglabs/loop.Commit=v0.29.0-beta -X github.com/lightninglabs/pool.Commit=v0.6.5-beta.0.20241015105339-044cb451b5df -X github.com/lightninglabs/taproot-assets.Commit=github.com" -o litd-debug github.com/lightninglabs/lightning-terminal/cmd/litd
go: updates to go.mod needed; to update it:
	go mod tidy
make: *** [Makefile:132: go-build] Error 1

then when I do go mod tidy as it says, I get

go: finding module for package github.com/lightningnetwork/lnd/graph/db/models
go: github.com/lightninglabs/lightning-terminal/cmd/litcli imports
	github.com/lightninglabs/taproot-assets/rfq imports
	github.com/lightningnetwork/lnd/graph/db/models: module github.com/lightningnetwork/lnd@latest found (v0.0.2), but does not contain package github.com/lightningnetwork/lnd/graph/db/models

I can build taproot-assets with this PR outside of litd though. I can also build litd with taproot-assets v0.5.1.

[ZZiigguurraatt]

OK, looks like I am having the same problem if I try to build litd with the taproot assets main branch tip too.

[ZZiigguurraatt]

@guggero Not sure if this issue is the cause?

https://github.com/lightninglabs/taproot-assets/blame/5290f9c3e811bb327a8f8a446d7b3519daf2b453/rfq/order.go#L16

[guggero]

@guggero Not sure if this issue is the cause?

https://github.com/lightninglabs/taproot-assets/blame/5290f9c3e811bb327a8f8a446d7b3519daf2b453/rfq/order.go#L16

This will be required first: https://github.com/lightninglabs/lightning-terminal/pull/998

[ZZiigguurraatt]

@guggero Not sure if this issue is the cause? https://github.com/lightninglabs/taproot-assets/blame/5290f9c3e811bb327a8f8a446d7b3519daf2b453/rfq/order.go#L16

This will be required first: lightninglabs/lightning-terminal#998

OK, so https://github.com/lightninglabs/taproot-assets/blame/5290f9c3e811bb327a8f8a446d7b3519daf2b453/rfq/order.go#L16 is not the cause, but instead, the fix for taproot-assets and other dependencies are basically missing that same fix?

[guggero]

The update to lnd 19 was already done in taproot-assets, which came with a bunch of changes. Those changes also need to be done in litd for things to be compile-time compatible. Which is what the PR I linked to does.

[ZZiigguurraatt]

OK, so no way to test this (and any other open taproot-assets) PR with lightning channels then until https://github.com/lightninglabs/lightning-terminal/pull/998 is merged?

[guggero]

Correct. I'm working on it.

[ZZiigguurraatt]

Now that https://github.com/lightninglabs/lightning-terminal/pull/998 is merged, I'm getting.

# github.com/lightninglabs/taproot-assets/tapchannel
../../taproot-assets/addinvoice-sats-amt/tapchannel/aux_funding_controller.go:2259:38: cannot use (*FundingController)(nil) (value of type *FundingController) as funding.AuxFundingController value in variable declaration: *FundingController does not implement funding.AuxFundingController (wrong type for method SendMessage)
		have SendMessage(msgmux.PeerMsg) bool
		want SendMessage("context".Context, msgmux.PeerMsg) bool
make: *** [Makefile:132: go-build] Error 1

However, I do have a cached version of branch lnd-19 at https://github.com/lightninglabs/lightning-terminal/commit/453835f42617c1a8db6027b7116bd79dacd12df4 which does compile. So, I'm wondering if there was some change to taproot-assets that was required to merge the latest version of branch lnd-19 in lightning-terminal and addinvoice-sats-amt (this PR) needs to be rebased on that?

[GeorgeTsagk]

@ZZiigguurraatt this PR needs a rebase, will soon update it

[GeorgeTsagk]

Rebased on latest main

[GeorgeTsagk]

Extracted a lot of the custom logic that resided inside AddInvoice to helpers in rfq/rfqmath packages

Also realized we don't need to apply the tolerance to the max units in the non-sats case, so made that part conditional too

Isolated the "small refactor" in the last 3 commits, will probably keep it like this to avoid aggressive force-pushes and have a more clean diff history.

PTAL @guggero @Roasbeef

[GeorgeTsagk]

Further abstracted away pieces of AddInvoice, thanks @guggero for the recommendation

[ZZiigguurraatt]

When specifying value/value_msat, would it be beneficial to return asset_amount in tapchannelrpc.AddInvoiceResponse so people don't have to calculate it from the accepted_buy_quote? Might just make it clearer to people what this is doing.

[guggero]

I pushed up a version that simplifies the commit structure, so we don't touch the same code multiple times. Should also make it easier to spot what exactly in the logic is changed (don't want to introduce a potential siphon or other kind of exploitable vulnerability here, so yes, I'm extra strict with reviewer friendliness of commit structure).

[guggero]

Litd test will succeed after merging https://github.com/lightninglabs/lightning-terminal/pull/1040.

[guggero]

Also fixes https://github.com/lightninglabs/taproot-assets/issues/1306.

[lightninglabs-deploy]

@roasbeef: review reminder @georgetsagk, remember to re-request review from reviewers when ready

[Roasbeef]

Seeing this failure in the litd itest;, not sure if related:

    assets_test.go:1428: Asking peer 03a024e1ee071b31d510d75a7794f979ed1131b1b021492f7613f82fda9530c84a for quote to buy assets to receive for invoice over 1 units; waiting up to 300s
    assets_test.go:1445: 
        	Error Trace:	/home/runner/work/taproot-assets/taproot-assets/lightning-terminal/itest/assets_test.go:1445
        	            				/home/runner/work/taproot-assets/taproot-assets/lightning-terminal/itest/litd_custom_channels_test.go:2163
        	            				/home/runner/work/taproot-assets/taproot-assets/lightning-terminal/itest/test_harness.go:103
        	            				/home/runner/work/taproot-assets/taproot-assets/lightning-terminal/itest/litd_test.go:87
        	Error:      	Error "rpc error: code = Unknown desc = error validating invoice amount: cannot create invoice for 1 asset units, as the minimal transportable amount is 20 units with the current rate of coefficient:\"5820600\" units/BTC" does not contain "cannot create invoice over 1 asset units, as the minimal transportable amount"
        	Test:       	TestLightningTerminal/custom_channels_liquidity
    harness.go:381: finished test: , start height=438, end height=483, mined blocks=45
    harness.go:403: !============================================!
        Too many blocks (45) mined in one test! Tips:
        1. break test into smaller individual tests, especially if this is a table-drive test.
        2. use smaller CSV via `--bitcoin.defaultremotedelay=1.`
        3. use smaller CLTV via `--bitcoin.timelockdelta=18.`
        4. remove unnecessary CloseChannel when test ends.
        5. use `CreateSimpleNetwork` for efficient channel creation.

[Roasbeef]

Ah I see this comment now , so does this just need a rebase now?

[guggero]

Litd itest is expected to fail now, because the referenced branch requires https://github.com/lightninglabs/taproot-assets/pull/1478 to be merged. So this will become stable again soon.