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

feat: Extend GetInfo RPC with LND info. #554

Closed
wants to merge 1 commit into from

Conversation

ben2077
Copy link
Contributor

@ben2077 ben2077 commented Oct 9, 2023

Added additional fields to the GetInfo RPC response to provide richer details about the blockchain and the LND node. This includes the LND node's public key (identity), its alias, the current block height, and the latest block's hash.

@ben2077 ben2077 mentioned this pull request Oct 9, 2023
@guggero guggero self-requested a review October 9, 2023 07:36
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this over! Needs a couple of small fixes but is otherwise almost ready to go!

rpcserver.go Outdated
@@ -286,10 +286,29 @@ func (r *rpcServer) DebugLevel(ctx context.Context,
func (r *rpcServer) GetInfo(context.Context,
*taprpc.GetInfoRequest) (*taprpc.GetInfoResponse, error) {

// Retrieve the best block hash and height from the chain backend.
blockHash, blockHeight, err := r.cfg.Lnd.ChainKit.GetBestBlock(context.Background())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the inbound context (just need to give it and the *taprpc.GetInfoRequest a parameter name). Here and below.

rpcserver.go Outdated
// Retrieve the best block hash and height from the chain backend.
blockHash, blockHeight, err := r.cfg.Lnd.ChainKit.GetBestBlock(context.Background())
if err != nil {
// Handle error appropriately
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't give any additional info and should just be removed, here and below.

rpcserver.go Outdated
@@ -308,6 +308,7 @@ func (r *rpcServer) GetInfo(context.Context,
NodeAlias: info.Alias,
BlockHeight: uint32(blockHeight),
BlockHash: blockHash.String(),
SyncStatus: info.SyncedToChain,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit should be squashed with the previous one.

@dstadulis dstadulis added the v0.3 label Oct 9, 2023
@dstadulis
Copy link
Collaborator

could include in a subsequent v0.3 RC if we get the requested changes

rpcserver.go Outdated
NodeAlias: info.Alias,
BlockHeight: uint32(blockHeight),
BlockHash: blockHash.String(),
SyncStatus: info.SyncedToChain,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to change SyncStatus to SyncedToChain. As a user, seeing "sync_status": true in the output is pretty vague. It's not clear whether this is related to the chain or graph or something new in tapd regarding universes.

… and block hash

Added additional fields to the GetInfo RPC response to provide richer details about the blockchain and the LND node. This includes the LND node's public key (identity), its alias, the current block height, and the latest block's hash.
@guggero
Copy link
Member

guggero commented Oct 16, 2023

Included a rebased version of the commit in this PR: #589 (keeping author attribution intact).

@guggero guggero closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants