Skip to content

Change block data request to get witness block#548

Closed
randomlogin wants to merge 1 commit into2140-dev:masterfrom
randomlogin:fix-witness-block-inventory
Closed

Change block data request to get witness block#548
randomlogin wants to merge 1 commit into2140-dev:masterfrom
randomlogin:fix-witness-block-inventory

Conversation

@randomlogin
Copy link
Contributor

Change block data request to get witness block
Add a small test which might be unnecessary.

block() was sending Block instead of WitnessBlock.

ProbabLy this was a regression introduced when the filter-control feature flag was removed (4ed9a55). The original code conditionally used Inventory::WitnessBlock under that feature; when the feature was removed the WitnessBlock path was lost and Inventory::Block became the unconditional default.

Encountered this issue when tried to use kyoto for ldk-node (lightningdevkit/ldk-node#821).

block() was sending Block instead of WitnessBlock.

Probaby this was a regression introduced when the `filter-control`
feature flag was removed (4ed9a55). The original code conditionally used
`Inventory::WitnessBlock` under that feature; when the feature was
removed the `WitnessBlock` path was lost and `Inventory::Block` became
the unconditional default.
@rustaceanrob
Copy link
Collaborator

Most on-chain clients will not be doing block validation, so this would be a waste of bandwidth for some users. Can we instead add this as a builder and Config option that is subsequently passed to the callsite?

@rustaceanrob
Copy link
Collaborator

I am going to merge #549 instead. May I have the email associated with your github account to add you as a commit co-author?

@randomlogin
Copy link
Contributor Author

I am going to merge #549 instead. May I have the email associated with your github account to add you as a commit co-author?

You can use randomlogin76 at gmail

@rustaceanrob
Copy link
Collaborator

Thanks, you are attributed in #549. Closing this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants