taprpc: extract as standalone Go submodule for WASM clients #1487

ffranr · 2025-04-24T17:01:34Z

Reformulate taprpc as a standalone Go submodule. This allows WASM clients to depend on a smaller, more focused module, rather than importing the entire taproot-assets package.

Related issue: lightninglabs/lightning-node-connect#113

Overall plan: lightninglabs/lightning-node-connect#113 (comment)

ffranr · 2025-04-24T17:03:02Z

This is a minimal replacement of #1090

coveralls · 2025-04-24T18:19:38Z

Pull Request Test Coverage Report for Build 14789890974

Details

5 of 151 (3.31%) changed or added relevant lines in 13 files are covered.
45 unchanged lines in 9 files lost coverage.
Overall coverage increased (+8.2%) to 36.958%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cmd/commands/addrs.go	0	1	0.0%
itest/assertions.go	0	1	0.0%
itest/utils.go	0	1	0.0%
rpcutils/rfq_marshal.go	0	2	0.0%
itest/multisig.go	0	3	0.0%
itest/oracle_harness.go	0	3	0.0%
server.go	0	3	0.0%
proof/courier.go	0	5	0.0%
rpcutils/price_oracle_marshal.go	1	11	9.09%
rpcutils/universe_marshal.go	0	12	0.0%

Files with Coverage Reduction	New Missed Lines	%
commitment/tap.go	1	71.59%
rpcserver.go	2	0.0%
tapgarden/planter.go	2	60.85%
tappsbt/create.go	2	26.74%
rfqmsg/records.go	3	64.11%
tapchannel/aux_leaf_signer.go	3	43.43%
address/address.go	6	67.47%
tapgarden/caretaker.go	10	68.31%
address/mock.go	16	88.24%

Totals
Change from base Build 14776808586:	8.2%
Covered Lines:	26469
Relevant Lines:	71619

💛 - Coveralls

ffranr · 2025-04-28T14:36:50Z

Overall plan: lightninglabs/lightning-node-connect#113 (comment)

server.go

guggero

Thanks for looking into this!
Looks pretty good, but have a couple of suggestions.

cmd/commands/addrs.go

guggero · 2025-04-29T06:42:47Z

docs/examples/basic-price-oracle/go.mod

@@ -20,7 +20,10 @@ require (
 	dario.cat/mergo v1.0.1 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
 	github.com/Microsoft/go-winio v0.6.1 // indirect
+	github.com/NebulousLabs/fastrand v0.0.0-20181203155948-6fb6489aac4e // indirect


While we're at it: can we please replace the github.com/lightninglabs/taproot-assets => ../../../ at the top with a fixed version so we don't have to update these two files with almost every PR?
I know it risks us forgetting to update it if we ever actually change anything that impacts the example. But the number of force pushes and CI cycles that can be avoided by forgetting to update these two files IMO is worth it.

I've pinned to the latest main commit of taproot-assets for now because that was the easiest to build. Once the next release is out, I will pin to that. See commit message for more. Let me know if there's a better strategy.

guggero · 2025-04-29T06:44:46Z

rpcutils/price_oracle_marshal_test.go

@@ -1,17 +1,18 @@
-package priceoraclerpc
+package rpcutils


Same as my previous comment, this would IMO better fit in the oracle package.

guggero · 2025-04-29T06:46:34Z

rpcutils/rfq_marshal.go

-// UnmarshalFixedPoint converts an RPC FixedPoint to a BigIntFixedPoint.
-func UnmarshalFixedPoint(fp *FixedPoint) (*rfqmath.BigIntFixedPoint, error) {
+// UnmarshalRfqFixedPoint converts an RPC FixedPoint to a BigIntFixedPoint.
+func UnmarshalRfqFixedPoint(fp *rfqrpc.FixedPoint) (*rfqmath.BigIntFixedPoint,


Looking at my previously proposed rule "put these functions into the package that defines the non-RPC type" should perhaps be a soft rule or "rule of thumb". Don't think placing this in the rfqmath package makes that much sense. But IMO it definitely makes sense in the rfq package.

taprpc/go.mod

Roasbeef

Overall concept ACK.

No strong feelings on if we go with the rpcutil package vs putting RPC serialization routines in select packages. The main pitfall to avoid as mentioned is taking care to not introduce circular dep cycles.

If main is the only package that imports this new one, then we should be ok. However if any other package imports it, then it'll likely be easy to run into a circular dep.

cmd/commands/addrs.go

taprpc/mintrpc/mint.swagger.json

ffranr · 2025-04-30T13:38:52Z

rpcserver.go can import anything, nothing depends on it. If we treated rpcutils similarly, then maybe we could move some marshalling functions from rpcserver.go to rpcutils.

Which might be useful for outside developers. And also slim down rpcserver.go.

For example:

unmarshalAddrEventStatus
unmarshalAssetBuyOrder
unmarshalAssetLeaf
unmarshalAssetSellOrder
unmarshalAssetSpecifier
unmarshalAssetSyncConfig
unmarshalCoinSelectType
UnmarshalGroupWitness
unmarshalLeafKey
unmarshalSyncTargets
UnmarshalUniID
UnmarshalUniProofType
unmarshalUniverseKey
unmarshalUniverseServer
unmarshalUniverseSyncType

marshalAddr
marshalAddrEvent
marshalAddrEventStatus
MarshalAssetFedSyncCfg
marshalAssetLeaf
marshalAssetSpecifier
marshalBatchState
marshalLeafKey
marshallReceiveAssetEvent
marshallRfqEvent
marshallSendAssetEvent
marshalMintingBatch
marshalMssmtNode
marshalMssmtProof
marshalOutboundParcel
marshalOutputProofDeliveryStatus
marshalOutputType
marshalPeerAcceptedBuyQuotes
marshalPeerAcceptedSellQuotes
marshalRpcBurn
marshalSeedling
marshalSeedlings
marshalSendEvent
marshalSprouts
MarshalUniID
MarshalUniProofType
marshalUniverseRoot
marshalUniverseServer
marshalUnsealedSeedling
marshalUnsealedSeedlings
marshalVerboseBatch

guggero

Nice, LGTM 🎉 (pending green CI which can be achieved through rebasing).

cmd/commands/addrs.go

Ideally, we would pin to a stable release of taproot-assets, as this is more useful for developers. However, release v0.5.1-alpha introduces logging changes that complicate integration. For now, we pin to the latest `main` commit and will update to the next stable release once it's available.

ffranr · 2025-05-01T21:38:44Z

Rebasing...

Our overall goal is to make taprpc as lean as possible so that it is not dependent on the rest of the `taproot-assets` module. Taprpc will then be made into a module that can be imported for lean WASM builds. With that in mind, we move gRPC marshalling functionality from the taprpc package to a new package called rpcutils.

Move price oracle related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

Move RFQ related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

Roasbeef · 2025-05-01T22:09:17Z

Needs another rebase?

Roasbeef · 2025-05-01T22:15:31Z

Looks like we still have some marshaling code here: https://github.com/lightninglabs/taproot-assets/blob/1741fd4a0e9ad57b7cbad33cc4235fb795f221db/taprpc/universerpc/marshal.go

Or is that intended?

Roasbeef

LGTM 🥦

Only comment is if we want to move over this marshal code into the new package created: https://github.com/lightninglabs/taproot-assets/blob/1741fd4a0e9ad57b7cbad33cc4235fb795f221db/taprpc/universerpc/marshal.go

Move universe related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

ffranr · 2025-05-01T22:48:49Z

LGTM 🥦

Only comment is if we want to move over this marshal code into the new package created: https://github.com/lightninglabs/taproot-assets/blob/1741fd4a0e9ad57b7cbad33cc4235fb795f221db/taprpc/universerpc/marshal.go

I've moved those marshalling functions into rpcutils also, to be consistent.

Convert taprpc into a separate Go module. This change allows WASM clients to depend only on this smaller module, avoiding the need to import all exported symbols from the taproot-assets package.

The Lightning Node Connect WASM client requires access to `perms.go` for RPC endpoint-related permissions. We move it into taprpc so that the WASM build only needs to depend on the taprpc module, without importing the entire taproot-assets module.

guggero · 2025-05-02T06:19:31Z

I pushed up a fix to the module creation commit and bumped some dependency versions. This removed the diff in the generated RPC code and should also prevent Dependabot to go crazy after we merge the PR.

ffranr added the gRPC label Apr 24, 2025

ffranr self-assigned this Apr 24, 2025

ffranr mentioned this pull request Apr 24, 2025

WASM build broken on litd version bump due to taproot-assets symbol bloat lightninglabs/lightning-node-connect#113

Closed

ffranr marked this pull request as draft April 24, 2025 17:13

ffranr force-pushed the taprpc-submodule branch from 4c0d4da to e87609b Compare April 28, 2025 14:35

ffranr marked this pull request as ready for review April 28, 2025 14:35

ffranr requested review from Roasbeef and guggero April 28, 2025 14:36

levmi added this to Taproot-Assets Project Board Apr 28, 2025

github-project-automation bot moved this to 🆕 New in Taproot-Assets Project Board Apr 28, 2025

levmi moved this from 🆕 New to 🏗 In progress in Taproot-Assets Project Board Apr 28, 2025

levmi moved this from 🏗 In progress to 👀 In review in Taproot-Assets Project Board Apr 28, 2025

Roasbeef reviewed Apr 28, 2025

View reviewed changes

server.go Show resolved Hide resolved

guggero reviewed Apr 29, 2025

View reviewed changes

Roasbeef reviewed Apr 29, 2025

View reviewed changes

cmd/commands/addrs.go Show resolved Hide resolved

taprpc/mintrpc/mint.swagger.json Outdated Show resolved Hide resolved

ffranr force-pushed the taprpc-submodule branch from e87609b to d68e675 Compare April 30, 2025 13:12

guggero approved these changes Apr 30, 2025

View reviewed changes

cmd/commands/addrs.go Show resolved Hide resolved

ffranr force-pushed the taprpc-submodule branch from d68e675 to c1acb87 Compare May 1, 2025 10:19

ffranr requested a review from Roasbeef May 1, 2025 11:53

ffranr added 3 commits May 1, 2025 23:01

rpcutils: move price oracle gRPC marshal functionality from taprpc

ef73f75

Move price oracle related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

rpcutils: move RFQ gRPC marshal functionality from taprpc

e41ddf2

Move RFQ related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

Roasbeef approved these changes May 1, 2025

View reviewed changes

ffranr force-pushed the taprpc-submodule branch from c1acb87 to 4ef9e01 Compare May 1, 2025 22:19

rpcutils: move universe gRPC marshal functionality from taprpc

3e21462

Move universe related gRPC marshalling functionality from the taprpc package to the rpcutils package as preparation for future modularization.

ffranr force-pushed the taprpc-submodule branch from 4ef9e01 to 6dc733e Compare May 1, 2025 22:48

ffranr added this pull request to the merge queue May 1, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2025

bhandras and others added 2 commits May 2, 2025 08:17

taprpc: convert taprpc to a Go submodule

0067c90

Convert taprpc into a separate Go module. This change allows WASM clients to depend only on this smaller module, avoiding the need to import all exported symbols from the taproot-assets package.

perms: move perms.go into taprpc

8418383

The Lightning Node Connect WASM client requires access to `perms.go` for RPC endpoint-related permissions. We move it into taprpc so that the WASM build only needs to depend on the taprpc module, without importing the entire taproot-assets module.

guggero force-pushed the taprpc-submodule branch from 6dc733e to 8418383 Compare May 2, 2025 06:18

guggero merged commit 5225722 into main May 2, 2025
17 of 18 checks passed

github-project-automation bot moved this from 👀 In review to ✅ Done in Taproot-Assets Project Board May 2, 2025

guggero deleted the taprpc-submodule branch May 2, 2025 07:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

taprpc: extract as standalone Go submodule for WASM clients #1487

taprpc: extract as standalone Go submodule for WASM clients #1487

ffranr commented Apr 24, 2025 •

edited

Loading

ffranr commented Apr 24, 2025

coveralls commented Apr 24, 2025 •

edited

Loading

ffranr commented Apr 28, 2025

guggero left a comment

guggero Apr 29, 2025

ffranr Apr 30, 2025

guggero Apr 29, 2025

guggero Apr 29, 2025

Roasbeef left a comment

ffranr commented Apr 30, 2025 •

edited

Loading

guggero left a comment

ffranr commented May 1, 2025

Roasbeef commented May 1, 2025

Roasbeef commented May 1, 2025

Roasbeef left a comment

ffranr commented May 1, 2025

guggero commented May 2, 2025

taprpc: extract as standalone Go submodule for WASM clients #1487

taprpc: extract as standalone Go submodule for WASM clients #1487

Conversation

ffranr commented Apr 24, 2025 • edited Loading

ffranr commented Apr 24, 2025

coveralls commented Apr 24, 2025 • edited Loading

Pull Request Test Coverage Report for Build 14789890974

Details

💛 - Coveralls

ffranr commented Apr 28, 2025

guggero left a comment

Choose a reason for hiding this comment

guggero Apr 29, 2025

Choose a reason for hiding this comment

ffranr Apr 30, 2025

Choose a reason for hiding this comment

guggero Apr 29, 2025

Choose a reason for hiding this comment

guggero Apr 29, 2025

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

ffranr commented Apr 30, 2025 • edited Loading

guggero left a comment

Choose a reason for hiding this comment

ffranr commented May 1, 2025

Roasbeef commented May 1, 2025

Roasbeef commented May 1, 2025

Roasbeef left a comment

Choose a reason for hiding this comment

ffranr commented May 1, 2025

guggero commented May 2, 2025

ffranr commented Apr 24, 2025 •

edited

Loading

coveralls commented Apr 24, 2025 •

edited

Loading

ffranr commented Apr 30, 2025 •

edited

Loading