Skip to content

Conversation

@dergoegge
Copy link
Member

@dergoegge dergoegge commented Dec 21, 2021

This PR addresses unresolved review comments from #17631.
This includes:

  • various style fix-ups
  • returning a more verbose error message for invalid header counts
  • removing superfluous rpc serializations flags for block filters
  • improving the test to include comparing the block filters returned from the rest with the ones returned from the getblockfilter RPC.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

Concept ACK. For reference, I've also opened #23599 to clean up the RPCTxSerializationFlags function.

I think the pull title and description can be modified to be self-contained. That is, following links is not needed to understand the changes.

@dergoegge dergoegge force-pushed the 17631_style_followup branch from c67f1d7 to 4523d28 Compare December 22, 2021 19:59
@dergoegge dergoegge changed the title #17631 followups rest: Expose block filters follow-ups Dec 22, 2021
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

tACK 4523d28

Style fix-ups and the new error message seem ok for me, I ran the test and it passed successfully.

@fanquake
Copy link
Member

cc @stickies-v @jnewbery

@jnewbery
Copy link
Contributor

jnewbery commented Jan 1, 2022

ACK 4523d28

Thanks for picking this up!

@fanquake fanquake merged commit 9d099b0 into bitcoin:master Jan 2, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 2, 2022
4523d28 [test] compare filter and header with the result of the getblockfilter RPC (Niklas Gögge)
3a2464f [rest] drop superfluous rpc serializations flags for block filters (Niklas Gögge)
064abd1 [rest] add a more verbose error message for invalid header counts (Niklas Gögge)
83b8f3a [refactor] various style fix-ups (Niklas Gögge)

Pull request description:

  This PR addresses unresolved review comments from [bitcoin#17631](bitcoin#17631).
  This includes:
  * various style fix-ups
  * returning a more verbose error message for invalid header counts
  * removing superfluous rpc serializations flags for block filters
  * improving the test to include comparing the block filters returned from the rest  with the ones returned from the `getblockfilter` RPC.

ACKs for top commit:
  jnewbery:
    ACK 4523d28
  brunoerg:
    tACK 4523d28

Tree-SHA512: 634e6b2ae3e1d5f31675a50cfe11a5e74bf5a51b9e7b512d9e18879bf2ed424fc0ac6ec633023f192e3ad12cf0c73b0b51de79dd7ec00844dba3e1493d823b8c
@bitcoin bitcoin locked and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants