search

lightninglabs / lightning-terminal

Lightning Terminal: Your Home for Lightning Liquidity
MIT License
487 stars 82 forks source link

litclient: add taprpc to registrations #571

Closed ViktorTigerstrom closed 1 year ago

ViktorTigerstrom commented 1 year ago

This PR adds the taprpc to the litclient's Registrations array, to ensure that the TaprootAssets callbacks are registered and available in the litclient.

ellemouton commented 1 year ago

This will need a PR in the taproot-assets repo first. The build is currently failing cause the JSON callback method used from the TAP code is only built if the js build tag is set.

So this line will need to be edited so that it doesnt add the build_tags=// +build js part and then you can run make protos which should regen the file to not have the // +build js line

levmi commented 1 year ago

cc @dstadulis who can help us prio any necessary change on the tapd side

ViktorTigerstrom commented 1 year ago

Opened a PR for the required fix in the taproot-assets repo, and updated this PR to use a forked version containing the fix.

Also updated this PR to register the assetwalletrpc universerpc JSON callbacks in the litclients Registrations array as well!

ViktorTigerstrom commented 1 year ago

Updated this PR to use the latest version of the main branch for the taproot-asset dependency to include the required fix. Also update the litclient Registrations to include mintrpc 🚀.

Also rebased on the latest version of master.

ellemouton commented 1 year ago

we might not want to include all the other changes in the taproot-assets code since the v0.2.0 release since it could have behaviour changes that the user is not expecting. Users would then be running an un-released version of taproot-assets if they were to run it via LiT (if we were to do another LiT release that did not include a Taproot-assets bump)

This is something that we might want to do quite often in LiT & I think we dont always want to have to wait for a release on the sub-server side just cause we want to include some changes on the code-structure level that wont actually have behaviour changes for the user. So I think we should decide on a pattern for including such changes.

My suggestion would be: we should create a v0.2.0-lit-0 tag (using taproot-assets as an example) in that repo which points to a commit that includes the released version (v0.2.0) plus any specific commits we require for Lit. In this case it would be just the two commits added here. The next time we add a commit there, we add a v0.2.0-lit-1 tag etc. Then on the next taproot-assets release, we just use the v0.2.1 tag again etc etc.

Keen to hear what @guggero things of this idea 🙏

guggero commented 1 year ago

Good point, @ellemouton. There weren't any braking changes merged in taproot-assets recently, only bug fixes. And v0.2.1 should be fairly close. But IMO it still makes sense to create a branch, so I did that and pushed the v0.2.0-lit-0 tag.

ellemouton commented 1 year ago

shweet! thanks @guggero 🙏 🚀

@ViktorTigerstrom - can you update this PR to point to the new tag? after that, this is g2g!

ViktorTigerstrom commented 1 year ago

Thanks for the great feedback @ellemouton! Makes a lot sense. And thanks for creating the tag so quickly @guggero!!

Updated the dependency to use the newv0.2.0-lit-0 tag with the latest push!