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

Added support for MSG_FILTERED_WITNESS_BLOCK messages. #313

Open
wants to merge 455 commits into
base: master
from

Conversation

Projects
None yet
@CodeShark

CodeShark commented May 7, 2017

After much deliberation and a few attempts at other approaches to provide a workable near-term sync mechanism for thin clients that require witness data, I decided to just go with the simplest short-term path with the least amount of complications expecting BIP37 to be completely replaced eventually - hopefully in the not-very-distant future.

I believe the approach of filtering blocks on the client is a simpler and superior approach for most use cases than requiring the server to perform the filtering. I believe @Roasbeef has something written up for this that he's using for lnd. I would love to see that approach used in Bitcoin Core as well.

But given the imminent SegWit activation on the Litecoin mainnet, I believe this solution will suffice for all essential use cases of BIP37 for now - and I don't believe it's worth the effort to try to make more complex additions to BIP37 since it will eventually be entirely replaced.

Peers can request MSG_FILTERED_WITNESS_BLOCK and will receive a merkleblock structure with transactions serialized with witnesses. The merkle proof for the witnesses is not supplied. This means that the client cannot verify that the witness data is what's actually in the block. However, the attack vectors here given the actual intended use cases seem extremely slim for several reasons:

  1. The witness data contains signatures which the client can still verify. Spoofing the witness would require supplying signatures that still redeem the output, meaning that only parties that can sign for the output could produce false witness data.

  2. In order to use BIP37 with any real degree of privacy and security, you need to connect to a trusted node. If this is your setup, adding merkle proofs for witnesses is an unnecessary complication.

  3. The foreseen intended use cases here are wallets that support multisignature scripts or scripts with multiple execution paths where you want to be able to check which signatures are provided or which execution path has been taken. In anticipated use cases, there is not much an attacker could gain from a transaction being signed in two different ways - and typically, the attacker would be easily identifiable.

harding and others added some commits Oct 17, 2016

Merge bitcoin#8943: Release notes: add info about segwit and null dum…
…my soft forks

bf86073 Release notes: correct segwit signalling period start conditions (David A. Harding)
2de93f0 Relase notes: correct segwit activation point (David A. Harding)
5f9c7b0 Release notes: add info about segwit and null dummy soft forks (David A. Harding)
Merge bitcoin#8947: Add historical release notes for v0.13.0
c9ffe90 Add historical release notes for v0.13.0 (Micha)
rpc: Generate auth cookie in hex instead of base64
Base64 contains '/', and the '/' character in credentials is problematic
for AuthServiceProxy which represents the RPC endpoint as an URI with
user and password embedded.

Closes bitcoin#8399.

Github-Pull: bitcoin#8858
Rebased-From: 1c80386
Remove bogus assert on number of oubound connections.
This value can be significantly higher if the users uses addnode

Github-Pull: bitcoin#8944
Rebased-From: 1ab21cf
Merge bitcoin#8960: doc: update 0.13.1 release note info on linux arm…
… builds

d179eed doc: update 0.13.1 release note info on linux arm builds [skip ci] (mruddy)
Be more aggressive in connecting to peers with relevant services.
Only allow skipping relevant services until there are four outbound
 connections up.

This avoids quickly filling up with peers lacking the relevant
 services when addrman has few or none of them.

Github-Pull: bitcoin#8949
Rebased-From: 9583477
Make dnsseed's definition of acute need include relevant services.
We normally prefer to connect to peers offering the relevant services.

If we're not connected to enough peers with relevant services, we
 probably don't know about them and could use dnsseed's help.

Github-Pull: bitcoin#8949
Rebased-From: 4630479
doc: Add missing credit to release notes
(Eric participated in Segwit work but has no direct commits, so should
be mentioned)
doc: Update blurb in release notes
Minor version, not major version.
Merge bitcoin#9012: release-notes: Update from blog draft
99f5cf1 release-notes: Update from blog draft (Luke Dashjr)
[net] Remove assert(nMaxInbound > 0)
nMaxInbound might very well be 0 or -1, if the user prefers to keep
a small number of maxconnections.

Note: nMaxInbound of -1 means that the user set maxconnections
to 8 or less, but we still want to keep an additional slot for
the feeler connection.

Github-Pull: bitcoin#9008
Rebased-From: fa1c3c2
release: bump required osx version to 10.8. Credit jonasschnelli.
libc++ on 10.7 causes too many issues.

See bitcoin#8577 for discussion/details.

Github-Pull: bitcoin#9015
Rebased-From: 339c4b6
Merge bitcoin#9022: Update release notes to mention dropping OS X 10.…
…7 support

1d12463 Update release notes for dropping osx 10.7 support (Michael Ford)

shaolinfry and others added some commits Jan 30, 2017

Merge pull request #279 from wangxinxi/master
M prefix added for script addresses
Uses built-in byte swap if available (Apple) and if bswap_XX is undef…
…ined.

Defers to pre-defined version if found (e.g. protobuf). For protobuf case, the definitions are identical and thus include order should not affect results.

Github-Pull: bitcoin#9366
Rebased-From: 815f414
Merge pull request #285 from thrasher-/master
Litecoin: Reset testnet (now testnet4)
Merge pull request #287 from shaolinfry/releasenotes
Start release notes for 0.13.3
Merge pull request #289 from thrasher-/master
Litecoin: Add testnet checkpoint and min chain work
Merge pull request #294 from thrasher-/master
Litecoin: Adjust nCheckDepth value
Merge pull request #296 from erasmospunk/master
Fix typo in chainparams.cpp
@losh11

This comment has been minimized.

Member

losh11 commented on fd66169 Apr 5, 2017

main.cpp:4161:23: warning: implicit conversion from 'long' to 'int' changes value from 4000000000 to -294967296
      [-Wconstant-conversion]
        nCheckDepth = 4000000000; // suffices until the year 19000
                    ~ ^~~~~~~~~~
1 warning generated.
  CXX      libbitcoin

Clang

thrasher- and others added some commits Apr 27, 2017

Merge pull request #307 from thrasher-/master
Litecoin: Adjust nCheckDepth and add Testnet DNS seeder
@shaolinfry

This comment has been minimized.

Member

shaolinfry commented May 7, 2017

References bitcoin#10350

@CodeShark CodeShark force-pushed the CodeShark:ltc-MSG_FILTERED_WITNESS_BLOCK_3 branch from 2df383a to cbf87f9 May 8, 2017

@shaolinfry shaolinfry force-pushed the litecoin-project:master branch from 9110405 to 50ebea0 Jun 28, 2017

@thrasher- thrasher- force-pushed the litecoin-project:master branch from 3c0fef0 to 303b13b Sep 21, 2017

@thrasher- thrasher- force-pushed the litecoin-project:master branch from f091164 to 845fc69 Feb 13, 2018

@thrasher- thrasher- force-pushed the litecoin-project:master branch from 07fe91d to 3567eff May 1, 2018

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