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

How to make sure node.GetBlocks only fetches segwit blocks? #568

Closed
nopara73 opened this Issue Oct 21, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@nopara73
Contributor

nopara73 commented Oct 21, 2018

We use Node.GetBlocks(IEnumerable<uint256> neededBlocks, CancellationToken cancellationToken) to receive blocks.

However one time @davex25 fetched a block that didn't have witness data. Our NodeRequirement is the following:

new NodeRequirement
{
	RequiredServices = NodeServices.Network,
	MinVersion = 70012 // ProtocolVersion_WITNESS_VERSION
});

I have some questions:

  • What should we do in the NodeRequirement to achieve the desired behavior?
  • Shouldn't MinVersion = 70012 // ProtocolVersion_WITNESS_VERSION be enough? If not, why?
  • Should I set NodeServices.NODE_WITNESS for RequiredServices instead?
  • Should I set MinProtocolCapabilities? I guess we should set it to new ProtocolCapabilities() { SupportGetBlock = true, SupportWitness = true } however this MinProtocolCapabilities seem to overlap with RequiredServices and MinVersion.
@nopara73

This comment has been minimized.

Contributor

nopara73 commented Oct 21, 2018

Also is NBitcoin works as Bitcoin Core in this case?

If someone sent it a stripped block (e.g. by lying about their support or whatnot) and it contained any segwit using transactions then it would get dropped and the peer disconnected-- basically treated like merkle tree malleability.

If NBitcoin should in theory work like that, then
-> we either set the NodeRequirement above improperly or
-> there's a bug in NBitcoin.

If NBitcoin should work like that, but it was just not implemented yet, I think we'll implement it.

@NicolasDorier

This comment has been minimized.

Member

NicolasDorier commented Oct 24, 2018

NBitcoin should not try to establish what is a valid block. That said, you can easily fix your issue by adding a filter to Node.Filters to your nodes. The Filter features provide you an extensibility point to process the message before it is forwarded to the MessageReceived. So you can make the check here.

@NicolasDorier

This comment has been minimized.

Member

NicolasDorier commented Oct 24, 2018

Actually nevermind, you should not do this if you use GetBlock.

@NicolasDorier

This comment has been minimized.

Member

NicolasDorier commented Oct 24, 2018

Actually you can sorry. So yeah, you would add a filter checking the block is valid, and if not you will disconnect the peer.

Either this, or you disconnect in the code which call GetBlock.

@nopara73

This comment has been minimized.

Contributor

nopara73 commented Oct 24, 2018

Thanks!

@nopara73 nopara73 closed this Oct 24, 2018

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