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(grpc): adding is pruned and pruning height in blockchain info API #1420

Merged
merged 7 commits into from
Jul 20, 2024

Conversation

kehiy
Copy link
Contributor

@kehiy kehiy commented Jul 16, 2024

@kehiy kehiy requested a review from b00f July 16, 2024 16:28
state/state.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
@kehiy
Copy link
Contributor Author

kehiy commented Jul 17, 2024

@Ja7ad Hi, change requests fixed, please review.

@kehiy kehiy added the www label Jul 17, 2024
Copy link
Collaborator

@b00f b00f left a comment

Choose a reason for hiding this comment

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

To be compatible with Bitcoin RPC, let's add these information to the blockchain-info. Check here: https://developer.bitcoin.org/reference/rpc/getblockchaininfo.html

state/state.go Show resolved Hide resolved
store/mock.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
www/grpc/proto/network.proto Outdated Show resolved Hide resolved
www/grpc/proto/network.proto Outdated Show resolved Hide resolved
www/grpc/proto/network.proto Outdated Show resolved Hide resolved
www/grpc/proto/network.proto Outdated Show resolved Hide resolved
@kehiy
Copy link
Contributor Author

kehiy commented Jul 17, 2024

@b00f Also, why we need to be compatible with Bitcoin RPC? We are not a fork or L2.

Also no bitcoin application uses Pactus. so whats the reason to be compatible?

store/interface.go Outdated Show resolved Hide resolved
www/grpc/proto/network.proto Outdated Show resolved Hide resolved
@b00f
Copy link
Collaborator

b00f commented Jul 19, 2024

I think pruning information fits better in BlockchainInfo rather than NodeInfo. Regarding Bitcoin compatibility, many developers are familiar with Bitcoin, so we should create something that looks familiar and is better organized.

@kehiy
Copy link
Contributor Author

kehiy commented Jul 19, 2024

@b00f Blokchain info contains the chain info which is shared, node info contains the info about one node which is not shared between all nodes.

www/grpc/proto/network.proto Outdated Show resolved Hide resolved
@kehiy kehiy changed the title feat(gRPC): adding is pruned and pruning height in node info API feat(grpc): adding is pruned and pruning height in blockchain info API Jul 20, 2024
@kehiy
Copy link
Contributor Author

kehiy commented Jul 20, 2024

Note: we decided that to move these info to blockchain info instead of node info since it will be mostly used there, using this info you can find you whether this node can provide info about old blocks or not. In this way caller can check this even if node info API is disabled.

Note2: we moved pruning height logic to store from state.

@b00f b00f merged commit 10acc39 into pactus-project:main Jul 20, 2024
11 checks passed
@kehiy kehiy deleted the nodeinfo branch July 20, 2024 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding Prune information to BlockchainInfo API
3 participants