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
WIP: Initial BIP157 support #668
Conversation
2392377
to
80fe70a
Compare
logger.error(s"Exiting") | ||
sys.error(message.toString()) | ||
// we might want to log the stack trace if message is an error | ||
val (processedMessage: Any, stackTrace: Option[Array[StackTraceElement]]) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: stackTraceOpt
maybeBoxed.getCause() match { | ||
case null => (maybeBoxed, None) | ||
case err: NotImplementedError => (err, Some(err.getStackTrace())) | ||
case other: Throwable => (other, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do other
and maybeBoxed
in the null
case not have a getStackTrace
member?
/** | ||
* Filter types for BIP157 block content filters | ||
* | ||
* @see [[https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki#block-filters BIP158]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I believe the comment should reference 158
def apply( | ||
blockHash: DoubleSha256Digest, | ||
filter: GolombFilter): CompactFilterMessage = { | ||
val filterBytes = filter.bytes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double-check that this doesn't already include the CompactSizeUInt
, I forget whether or not it does
filterType: FilterType, | ||
stopHash: DoubleSha256Digest, | ||
previousFilterHeader: DoubleSha256Digest, | ||
filterHashes: Vector[DoubleSha256Digest]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should create a more specific type RawFilterHeader
that wraps a DoubleSha256Digest
(and have FilterHeader::hash
return one of these)
@@ -82,7 +95,7 @@ sealed abstract class ServiceIdentifier extends NetworkElement { | |||
val innerText = | |||
if (nodeNone) "none" | |||
else | |||
s"network=$nodeNetwork, getUtxo=$nodeGetUtxo, bloom=$nodeBloom, witness=$nodeWitness, xthin=$nodeXthin, networkLimited=$nodeNetworkLimited" | |||
s"network=$nodeNetwork, compactFilters=$nodeCompactFilters getUtxo=$nodeGetUtxo, bloom=$nodeBloom, witness=$nodeWitness, xthin=$nodeXthin, networkLimited=$nodeNetworkLimited" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing comma
@@ -143,6 +156,8 @@ object ServiceIdentifier extends Factory[ServiceIdentifier] { | |||
*/ | |||
val NODE_XTHIN: ServiceIdentifier = ServiceIdentifier(1 << 4) | |||
|
|||
val NODE_COMPACT_FILTERS = ServiceIdentifier(1 << 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc
val (hashes, remainingBytes) = loop(Vector.empty, filterHashesBytes) | ||
|
||
assert(hashes.length == filterHashesLength.toInt) | ||
assert(remainingBytes.isEmpty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is it nicer to use require
with error messages?
import org.bitcoins.core.p2p.GetCompactFilterHeadersMessage | ||
import scodec.bits.ByteVector | ||
|
||
object RawGetcompactFilterHeadersMessageSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
@@ -37,6 +46,24 @@ class DataMessageHandler(callbacks: SpvNodeCallbacks, chainHandler: ChainApi)( | |||
peerMsgSender: PeerMessageSender): Future[Unit] = { | |||
|
|||
payload match { | |||
case checkpoint: CompactFilterCheckPointMessage => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping: TODO
This was closed in favor of #695 |
In this PR we:
To test this PR/do dev work on BIP157 you need to compile Jimpo's fork of Core, PR/branch is here: bitcoin/bitcoin#16442
TODO:
getcfheaders
andcfheaders
messages