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

fix verbocity in getblock command #1232

Closed

Conversation

reinerRubin
Copy link

"getblock" command new verbosity levels were implemented in the BTC-core 0.15.0 [1 (see "The verbose argument of getblock")]. So you can easily get a block within all txs. It is a very useful feature.

But unfortunately rpcclient does not support a new format, when lot of people use "rpcclient" with standart bitcoin-core.

It's yet another pr. But I tried to add some tests, keep rpcserverhelp.go and rpcserver in an working state.

1 - https://github.com/bitcoin/bitcoin/blob/3c6286873e50248717afd7c56c664cee069c76fa/doc/release-notes/release-notes-0.15.0.md


issue: #1230
new pr: #1231
old pr: #1112

Copy link

@Ronmi Ronmi left a comment

Choose a reason for hiding this comment

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

Looks good. I can confirm it works with new bitcoind, and new codes are covered in test cases. Maybe we need some feedback from core teams, to decide whether we should share common fields between the types or just make a copy like I did.

@reinerRubin
Copy link
Author

Cool. How can I contact or cast core team members? It looks like there are a lot of open pr here.

I don't mind a copy. It will also remove extra logic from "btcjson/help.go". But "verbose == 1" and "verbose == 2" responses are similiar except the one field (tx). So I thought that the "rpcserver.go/getblock" would be repetitive.

@Ronmi
Copy link

Ronmi commented Jul 30, 2018

Totally agree. I have no idea how to contact core devs, and I believe they have some conventions about merging similar types. So maybe our best bet is waiting (?)

@reinerRubin
Copy link
Author

reinerRubin commented Jul 30, 2018

Yep. I am going to wait and the then cast sombody from people.

I am afraid that the project is stagnating a little.

@reinerRubin
Copy link
Author

@jrick, hello.
Can you review the pr? Or can you tell me about the project status? Should I just wait?

Sorry for being annoying, but it's hard to wait without any knowledge about the current situation.

@galileo-pkm
Copy link

@reinerRubin Works a treat, tested with bitcoind.
Saves me from running repeated calls to GetRawTransactionVerbose and the trashing of disks by bitcoind is gone.

@galileo-pkm
Copy link

@reinerRubin
There is an issue with this branch, that is not present in the latest master. I'm not sure how the change got introduced, did not have the time to investigate.
btcd/chaincfg/params.go line 382
RegressionNetParams: Bech32HRPSegwit should be "bcrt" not "tb"

@reinerRubin
Copy link
Author

reinerRubin commented Aug 24, 2018

@galileo-pkm, I have done a rebase and "Bech32HRPSegwit" is correct now. The rebase could break your lock version (dep, glide, etc.), so be careful. But I think it is acceptable for pr branches.

@galileo-pkm
Copy link

Looks good, Thanks
Hopefully someone will merge this PR, but considering how other, much simpler are sitting in a limbo ...

@phewitt
Copy link

phewitt commented Sep 24, 2018

Would love to see this get merged as well!

@connorwstein
Copy link

@Roasbeef can we merge this?

@durango
Copy link

durango commented Jul 13, 2019

@Roasbeef @davecgh any chance of merging this simple PR?

@jakesylvestre
Copy link
Collaborator

jakesylvestre commented Mar 4, 2020

@jcvernaleo (as per #1530)

Linked Issues:
#1230

Not having this prevents #1529 from being merged

@jcvernaleo
Copy link
Member

I'm a little confused at the relationship between this and #1529. is this one needed for #1529? Looks like a lot of it is redundant between the two.

@jakesylvestre
Copy link
Collaborator

yeah - your correct here. I misread the test output- #1529 is a newer version of this which adds more params

@jcvernaleo
Copy link
Member

Closing since #1529 supersedes this.

@jcvernaleo jcvernaleo closed this Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants