chainkit: expose getblockheader #8111

jharveyb · 2023-10-24T21:32:16Z

Change Description

Description of change / link to associated issue.

Addresses lightninglabs/taproot-assets#315.

Expose the GetBlockHeader call from ChainKit. This was already available from each possible chain.Interface, but not available for users of ChainKit; a similar call was added to NeutrinoKit, but that would not work for users not running with a Neutrino backend.

Steps to Test

Steps for reviewers to follow to test the change.

Compile and run lncli chain getbestblock to get a block hash. Use that as an argument for lncli chain getblockheader and a hex block header should be returned.

Pull Request Checklist

Testing

Your PR passes all CI checks.
Tests covering the positive and negative (error paths) are included.
Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
Commits follow the Ideal Git Commit Structure.
Any new logging statements use an appropriate subsystem and logging level.
There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

jharveyb · 2023-10-24T21:33:17Z

Not sure why so many protos were changed with this.

Roasbeef

Useful RPC call! Straight forward changes.

Comments mainly about getting the commits to build, and also seeing why you're generating diff protos.

lnrpc/chainrpc/chainkit.proto

lnrpc/signrpc/signer.swagger.json

lnrpc/chainrpc/chain_server.go

lnwallet/mock.go

Roasbeef · 2023-10-24T23:07:12Z

If I build using make rpc, then I get a pretty sizeable diff, similar to what CI shows, stuff like:

-               var err error
-               var annotatedContext context.Context
-               annotatedContext, err = runtime.AnnotateIncomingContext(ctx, mux, req, "/lnrpc.Lightning/AddInvoice", runtime.WithHTTPPathPattern("/v1/invoices"))
+               rctx, err := runtime.AnnotateIncomingContext(ctx, mux, req, "/lnrpc.Lightning/AddInvoice", runtime.WithHTTPPathPattern("/v1/invoices"))
                if err != nil {
                        runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
                        return
                }
-               resp, md, err := local_request_Lightning_AddInvoice_0(annotatedContext, inboundMarshaler, server, req, pathParams)
+               resp, md, err := local_request_Lightning_AddInvoice_0(rctx, inboundMarshaler, server, req, pathParams)
                md.HeaderMD, md.TrailerMD = metadata.Join(md.HeaderMD, stream.Header()), metadata.Join(md.TrailerMD, stream.Trailer())
-               annotatedContext = runtime.NewServerMetadataContext(annotatedContext, md)
+               ctx = runtime.NewServerMetadataContext(ctx, md)
                if err != nil {
-                       runtime.HTTPError(annotatedContext, mux, outboundMarshaler, w, req, err)
+                       runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
                        return
                }

jharveyb · 2023-10-25T15:04:47Z

If I build using make rpc, then I get a pretty sizeable diff, similar to what CI shows, stuff like:

-               var err error
-               var annotatedContext context.Context
-               annotatedContext, err = runtime.AnnotateIncomingContext(ctx, mux, req, "/lnrpc.Lightning/AddInvoice", runtime.WithHTTPPathPattern("/v1/invoices"))
+               rctx, err := runtime.AnnotateIncomingContext(ctx, mux, req, "/lnrpc.Lightning/AddInvoice", runtime.WithHTTPPathPattern("/v1/invoices"))
                if err != nil {
                        runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
                        return
                }
-               resp, md, err := local_request_Lightning_AddInvoice_0(annotatedContext, inboundMarshaler, server, req, pathParams)
+               resp, md, err := local_request_Lightning_AddInvoice_0(rctx, inboundMarshaler, server, req, pathParams)
                md.HeaderMD, md.TrailerMD = metadata.Join(md.HeaderMD, stream.Header()), metadata.Join(md.TrailerMD, stream.Trailer())
-               annotatedContext = runtime.NewServerMetadataContext(annotatedContext, md)
+               ctx = runtime.NewServerMetadataContext(ctx, md)
                if err != nil {
-                       runtime.HTTPError(annotatedContext, mux, outboundMarshaler, w, req, err)
+                       runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
                        return
                }

Looks like the RPC build step is using a bookworm Docker image, with a newer protobuf compiler?

Swtich to bookworm:

Digest: sha256:445f34008a77b0b98bf1821bf7ef5e37bb63cc42d22ee7c21cc17041070d134f
Status: Downloaded newer image for golang:1.21.0-alpine
Building protobuf compiler docker image...
#0 building with "default" instance using docker driver

#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 1.26kB done
#2 DONE 0.0s

#3 [internal] load metadata for docker.io/library/golang:1.21.0-bookworm
#3 ...

#4 [auth] library/golang:pull token for registry-1.docker.io
#4 DONE 0.0s

#3 [internal] load metadata for docker.io/library/golang:1.21.0-bookworm
#3 DONE 1.0s

#5 [1/4] FROM docker.io/library/golang:1.21.0-bookworm@sha256:640f192f24852cd875514e18a01977ae87692dca466abed7705ad3c4670b6993

Compiler install:

 #6 3.259 After this operation, 252 MB of additional disk space will be used.
#6 3.259 Get:1 http://deb.debian.org/debian bookworm/main amd64 libicu72 amd64 72.1-3 [9376 kB]
#6 3.372 Get:2 http://deb.debian.org/debian bookworm/main amd64 libxml2 amd64 2.9.14+dfsg-1.3~deb12u1 [687 kB]
#6 3.372 Get:3 http://deb.debian.org/debian bookworm/main amd64 libz3-4 amd64 4.8.12-3.1 [7216 kB]
#6 3.425 Get:4 http://deb.debian.org/debian bookworm/main amd64 libllvm14 amd64 1:14.0.6-12 [21.8 MB]
#6 3.575 Get:5 http://deb.debian.org/debian bookworm/main amd64 libclang-cpp14 amd64 1:14.0.6-12 [11.1 MB]
#6 3.651 Get:6 http://deb.debian.org/debian bookworm/main amd64 clang-format-14 amd64 1:14.0.6-12 [75.0 kB]
#6 3.652 Get:7 http://deb.debian.org/debian bookworm/main amd64 clang-format amd64 1:14.0-55.7~deb12u1 [5256 B]
#6 3.652 Get:8 http://deb.debian.org/debian bookworm/main amd64 zlib1g-dev amd64 1:1.2.13.dfsg-1 [916 kB]
#6 3.659 Get:9 http://deb.debian.org/debian bookworm/main amd64 libprotobuf32 amd64 3.21.12-3 [932 kB]
#6 3.666 Get:10 http://deb.debian.org/debian bookworm/main amd64 libprotobuf-lite32 amd64 3.21.12-3 [261 kB]
#6 3.667 Get:11 http://deb.debian.org/debian bookworm/main amd64 libprotobuf-dev amd64 3.21.12-3 [1283 kB]
#6 3.676 Get:12 http://deb.debian.org/debian bookworm/main amd64 libprotoc32 amd64 3.21.12-3 [829 kB]
#6 3.682 Get:13 http://deb.debian.org/debian bookworm/main amd64 protobuf-compiler amd64 3.21.12-3 [83.9 kB]
#6 3.803 debconf: delaying package configuration, since apt-utils is not installed
#6 3.826 Fetched 54.6 MB in 0s (118 MB/s)
#6 3.840 Selecting previously unselected package libicu72:amd64.
#6 3.840 (Reading database ...

rpc-check target detects incorrect compiler version:

 Verifying protos.
cd ./lnrpc; ../scripts/check-rest-annotations.sh
if test -n "$(git status --porcelain)"; then echo "Protos not properly formatted or not compiled with v3.4.0"; git status; git diff; exit 1; fi
Protos not properly formatted or not compiled with v3.4.0

guggero · 2023-10-25T15:10:27Z

Ah, the error message in the rpc-check target is out of date. Just run make rpc, commit the changes and the make rpc-check shouldn't fail (it basically detects a dirty git working directory, so you can only run it after you committed the changes).

jharveyb · 2023-10-26T18:37:21Z

Ah, the error message in the rpc-check target is out of date. Just run make rpc, commit the changes and the make rpc-check shouldn't fail (it basically detects a dirty git working directory, so you can only run it after you committed the changes).

This worked, and the issue with testing before is from not updating the protos and chainkit.yaml at the same time.

Added a CLI command and itest + release note change, should be ready now.

guggero

utACK, LGTM (pending proper CI run with acceptable level of green).

lnrpc/chainrpc/chainkit.proto

cmd/lncli/neutrino_active.go

docs/release-notes/release-notes-0.17.1.md

jharveyb · 2023-10-26T20:20:48Z

Addressed feedback; the latst windows itest failure looked unrelated.

The linter failures are for 3 81-char lines, which are similar to other 81-char lines in those files and IIUC can't cleanly be shortened according to our style guidelines.

Roasbeef · 2023-10-26T22:08:45Z

The linter failures are for 3 81-char lines, which are similar to other 81-char lines in those files and IIUC can't cleanly be shortened according to our style guidelines.

Line ref?

Roasbeef

LGTM 🌷

Pending CI (just approved again)

docs/release-notes/release-notes-0.17.1.md

contractcourt/channel_arbitrator_test.go

lnwallet/btcwallet/blockchain.go

guggero

tACK, LGTM 🎉

Roasbeef requested changes Oct 24, 2023

View reviewed changes

dstadulis mentioned this pull request Oct 26, 2023

[bug]: neutrino header verifier times out on mainnet proof verification/sync lightninglabs/taproot-assets#613

Closed

jharveyb force-pushed the chainkit_add_getblockheader branch from a18b068 to 3e8a50c Compare October 26, 2023 18:36

jharveyb requested a review from Roasbeef October 26, 2023 18:42

guggero reviewed Oct 26, 2023

View reviewed changes

lnrpc/chainrpc/chainkit.proto Outdated Show resolved Hide resolved

cmd/lncli/neutrino_active.go Show resolved Hide resolved

docs/release-notes/release-notes-0.17.1.md Outdated Show resolved Hide resolved

chainrpc: update proto to expose GetBlockHeader

7a8803e

jharveyb force-pushed the chainkit_add_getblockheader branch from 3e8a50c to 3cc350b Compare October 26, 2023 20:19

Roasbeef added this to the v0.17.1 milestone Oct 26, 2023

Roasbeef approved these changes Oct 26, 2023

View reviewed changes

Roasbeef reviewed Oct 26, 2023

View reviewed changes

docs/release-notes/release-notes-0.17.1.md Outdated Show resolved Hide resolved

jharveyb force-pushed the chainkit_add_getblockheader branch from 3cc350b to 4ba332c Compare October 26, 2023 23:07

guggero reviewed Oct 27, 2023

View reviewed changes

contractcourt/channel_arbitrator_test.go Outdated Show resolved Hide resolved

lnwallet/btcwallet/blockchain.go Outdated Show resolved Hide resolved

jharveyb added 4 commits October 27, 2023 11:05

multi: add GetBlockHeader to BlockChainIO

6edd1e1

chainrpc+lncli: expose GetBlockHeader + add to CLI

c20e487

itest: add GetBlockHeader to ChainKit tests

c5d041f

doc: add release notes for 8111

459a60e

jharveyb force-pushed the chainkit_add_getblockheader branch from 4ba332c to 459a60e Compare October 27, 2023 15:07

guggero approved these changes Oct 27, 2023

View reviewed changes

guggero merged commit 3b7cda9 into lightningnetwork:master Oct 27, 2023
22 of 25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chainkit: expose getblockheader #8111

chainkit: expose getblockheader #8111

jharveyb commented Oct 24, 2023 •

edited

Loading

jharveyb commented Oct 24, 2023

Roasbeef left a comment

Roasbeef commented Oct 24, 2023

jharveyb commented Oct 25, 2023

guggero commented Oct 25, 2023

jharveyb commented Oct 26, 2023

guggero left a comment

jharveyb commented Oct 26, 2023

Roasbeef commented Oct 26, 2023

Roasbeef left a comment

guggero left a comment

chainkit: expose getblockheader #8111

chainkit: expose getblockheader #8111

Conversation

jharveyb commented Oct 24, 2023 • edited Loading

Change Description

Steps to Test

Pull Request Checklist

Testing

Code Style and Documentation

jharveyb commented Oct 24, 2023

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef commented Oct 24, 2023

jharveyb commented Oct 25, 2023

guggero commented Oct 25, 2023

jharveyb commented Oct 26, 2023

guggero left a comment

Choose a reason for hiding this comment

jharveyb commented Oct 26, 2023

Roasbeef commented Oct 26, 2023

Roasbeef left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

jharveyb commented Oct 24, 2023 •

edited

Loading