From 13a32811aa75f32b2c18f5a0b5cf290018176fa6 Mon Sep 17 00:00:00 2001 From: kdeme Date: Wed, 26 Jun 2019 13:19:59 +0200 Subject: [PATCH] Fix leading zeroes bug + add tests from geth and parity --- eth/p2p/rlpx_protocols/whisper_protocol.nim | 22 ++++----- tests/p2p/test_shh.nim | 50 +++++++++++++++++---- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/eth/p2p/rlpx_protocols/whisper_protocol.nim b/eth/p2p/rlpx_protocols/whisper_protocol.nim index 6c6c319..91370a1 100644 --- a/eth/p2p/rlpx_protocols/whisper_protocol.nim +++ b/eth/p2p/rlpx_protocols/whisper_protocol.nim @@ -155,12 +155,12 @@ proc leadingZeroBits(hash: MDigest): int = result += countLeadingZeroBits(h) break -proc calcPow(size, ttl: uint64, hash: Hash): float64 = +proc calcPow*(size, ttl: uint64, hash: Hash): float64 = ## Whisper proof-of-work is defined as the best bit of a hash divided by ## encoded size and time-to-live, such that large and long-lived messages get ## penalized - let bits = leadingZeroBits(hash) + 1 + let bits = leadingZeroBits(hash) return pow(2.0, bits.float64) / (size.float64 * ttl.float64) proc topicBloom*(topic: Topic): Bloom = @@ -431,7 +431,7 @@ proc valid*(self: Envelope, now = epochTime()): bool = proc len(self: Envelope): int = 20 + self.data.len -proc toShortRlp(self: Envelope): Bytes = +proc toShortRlp*(self: Envelope): Bytes = ## RLP-encoded message without nonce is used during proof-of-work calculations rlp.encodeList(self.expiry, self.ttl, self.topic, self.data) @@ -458,7 +458,7 @@ proc minePow*(self: Envelope, seconds: float, bestBitTarget: int = 0): (uint64, tmp.update(i.toBE()) # XXX:a random nonce here would not leak number of iters let hash = tmp.finish() - let zeroBits = leadingZeroBits(hash) + 1 + let zeroBits = leadingZeroBits(hash) if zeroBits > bestBit: # XXX: could also compare hashes as numbers instead bestBit = zeroBits result = (i, hash) @@ -494,7 +494,7 @@ proc initMessage*(env: Envelope, powCalc = true): Message = if powCalc: result.hash = env.calcPowHash() result.pow = calcPow(result.env.len.uint32, result.env.ttl, result.hash) - trace "Message PoW", pow = result.pow + trace "Message PoW", pow = result.pow.formatFloat(ffScientific) proc hash*(msg: Message): hashes.Hash = hash(msg.hash.data) @@ -515,16 +515,18 @@ proc allowed*(msg: Message, config: WhisperConfig): bool = return true -# NOTE: PoW calculations are different from go-ethereum implementation, -# which is not conform EIP-627. -# See here: https://github.com/ethereum/go-ethereum/issues/18070 -# However, this implementation is also not conform EIP-627 as we do not use the -# size of the RLP-encoded envelope, but the size of the envelope object itself. +# NOTE: Hashing and leading zeroes calculation is now the same between geth, +# parity and this implementation. +# However, there is still a difference in the size calculation. +# See also here: https://github.com/ethereum/go-ethereum/pull/19753 +# This implementation is not conform EIP-627 as we do not use the size of the +# RLP-encoded envelope, but the size of the envelope object itself. # This is done to be able to correctly calculate the bestBitTarget. # Other options would be: # - work directly with powTarget in minePow, but this requires recalculation of # rlp size + calcPow # - Use worst case size of envelope nonce +# - Mine PoW for x interval, calcPow of best result, if target not met .. repeat proc sealEnvelope(msg: var Message, powTime: float, powTarget: float): bool = let size = msg.env.len if powTarget > 0: diff --git a/tests/p2p/test_shh.nim b/tests/p2p/test_shh.nim index 0c89370..25aac94 100644 --- a/tests/p2p/test_shh.nim +++ b/tests/p2p/test_shh.nim @@ -186,12 +186,44 @@ let data: repeat(byte 9, 256), nonce: 1010102) suite "Whisper envelope": - test "should use correct fields for pow hash": - # XXX checked with parity, should check with geth too - found a potential bug - # in parity while playing with it: - # https://github.com/paritytech/parity-ethereum/issues/9625 - check $calcPowHash(env0) == - "A13B48480AEB3123CD2358516E2E8EE9FCB0F4CB37E68CD09FDF7F9A7E14767C" + + proc hashAndPow(env: Envelope): (string, float64) = + # This is the current implementation of go-ethereum + let size = env.toShortRlp().len().uint32 + # This is our current implementation in `whisper_protocol.nim` + # let size = env.len().uint32 + # This is the EIP-627 specification + # let size = env.toRlp().len().uint32 + let hash = env.calcPowHash() + ($hash, calcPow(size, env.ttl, hash)) + + test "PoW calculation leading zeroes tests": + # Test values from Parity, in message.rs + let testHashes = [ + # 256 leading zeroes + "0x0000000000000000000000000000000000000000000000000000000000000000", + # 255 leading zeroes + "0x0000000000000000000000000000000000000000000000000000000000000001", + # no leading zeroes + "0xff00000000000000000000000000000000000000000000000000000000000000" + ] + check: + calcPow(1, 1, Hash.fromHex(testHashes[0])) == + 115792089237316200000000000000000000000000000000000000000000000000000000000000.0 + calcPow(1, 1, Hash.fromHex(testHashes[1])) == + 57896044618658100000000000000000000000000000000000000000000000000000000000000.0 + calcPow(1, 1, Hash.fromHex(testHashes[2])) == 1.0 + + # Test values from go-ethereum whisperv6 in envelope_test + var env = Envelope(ttl: 1, data: @[byte 0xde, 0xad, 0xbe, 0xef]) + # PoW calculation with no leading zeroes + env.nonce = 100000 + check hashAndPoW(env) == ("A788E02A95BFC673709E97CA81E39CA903BAD5638D3388964C51EB64952172D6", + 0.07692307692307693) + # PoW calculation with 8 leading zeroes + env.nonce = 276 + check hashAndPoW(env) == ("00E2374C6353C243E4073E209A7F2ACB2506522AF318B3B78CF9A88310A2A11C", + 19.692307692307693) test "should validate and allow envelope according to config": let ttl = 1'u32 @@ -349,15 +381,15 @@ suite "Whisper filter": # this message has a PoW of 0.02962962962962963, number should be updated # in case PoW algorithm changes or contents of padding, payload, topic, etc. # update: now with NON rlp encoded envelope size the PoW of this message is - # 0.02898550724637681 + # 0.014492753623188406 let msg = prepFilterTestMsg(topic = topic, padding = padding) var filters = initTable[string, Filter]() let filterId1 = filters.subscribeFilter( - newFilter(topics = @[topic], powReq = 0.02898550724637681)) + newFilter(topics = @[topic], powReq = 0.014492753623188406)) filterId2 = filters.subscribeFilter( - newFilter(topics = @[topic], powReq = 0.02898550724637682)) + newFilter(topics = @[topic], powReq = 0.014492753623188407)) notify(filters, msg)