-
Notifications
You must be signed in to change notification settings - Fork 20.4k
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
whisper: new protocol-level message codes #15802
Changes from 4 commits
347fcb3
787054a
becaadd
5d71ee3
e33ef22
c27bade
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,15 +113,32 @@ func (api *PublicWhisperAPI) Info(ctx context.Context) Info { | |
// SetMaxMessageSize sets the maximum message size that is accepted. | ||
// Upper limit is defined by MaxMessageSize. | ||
func (api *PublicWhisperAPI) SetMaxMessageSize(ctx context.Context, size uint32) (bool, error) { | ||
return true, api.w.SetMaxMessageSize(size) | ||
err := api.w.SetMaxMessageSize(size) | ||
if err != nil { | ||
return false, err | ||
} | ||
return true, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the error is non-nil, the caller will not see the return value. You can use return true, err. |
||
} | ||
|
||
// SetMinPow sets the minimum PoW for a message before it is accepted. | ||
// SetMinPow sets the minimum PoW, and notifies the peers. | ||
func (api *PublicWhisperAPI) SetMinPoW(ctx context.Context, pow float64) (bool, error) { | ||
return true, api.w.SetMinimumPoW(pow) | ||
err := api.w.SetMinimumPoW(pow) | ||
if err != nil { | ||
return false, err | ||
} | ||
return true, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing as above |
||
} | ||
|
||
// SetBloomFilter sets the new value of bloom filter, and notifies the peers. | ||
func (api *PublicWhisperAPI) SetBloomFilter(ctx context.Context, bloom hexutil.Bytes) (bool, error) { | ||
err := api.w.SetBloomFilter(bloom) | ||
if err != nil { | ||
return false, err | ||
} | ||
return true, nil | ||
} | ||
|
||
// MarkTrustedPeer marks a peer trusted. , which will allow it to send historic (expired) messages. | ||
// MarkTrustedPeer marks a peer trusted, which will allow it to send historic (expired) messages. | ||
// Note: This function is not adding new nodes, the node needs to exists as a peer. | ||
func (api *PublicWhisperAPI) MarkTrustedPeer(ctx context.Context, enode string) (bool, error) { | ||
n, err := discover.ParseNode(enode) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,6 @@ import ( | |
) | ||
|
||
const ( | ||
EnvelopeVersion = uint64(0) | ||
ProtocolVersion = uint64(6) | ||
ProtocolVersionStr = "6.0" | ||
ProtocolName = "shh" | ||
|
@@ -52,11 +51,12 @@ const ( | |
paddingMask = byte(3) | ||
signatureFlag = byte(4) | ||
|
||
TopicLength = 4 | ||
signatureLength = 65 | ||
aesKeyLength = 32 | ||
AESNonceLength = 12 | ||
keyIdSize = 32 | ||
TopicLength = 4 // in bytes | ||
signatureLength = 65 // in bytes | ||
aesKeyLength = 32 // in bytes | ||
AESNonceLength = 12 // in bytes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really just meant for the |
||
keyIdSize = 32 // in bytes | ||
bloomFilterSize = 64 // in bytes | ||
|
||
MaxMessageSize = uint32(10 * 1024 * 1024) // maximum accepted size of a message. | ||
DefaultMaxMessageSize = uint32(1024 * 1024) | ||
|
@@ -68,10 +68,8 @@ const ( | |
expirationCycle = time.Second | ||
transmissionCycle = 300 * time.Millisecond | ||
|
||
DefaultTTL = 50 // seconds | ||
SynchAllowance = 10 // seconds | ||
|
||
EnvelopeHeaderLength = 20 | ||
DefaultTTL = 50 // seconds | ||
DefaultSyncAllowance = 10 // seconds | ||
) | ||
|
||
type unknownVersionError uint64 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,13 +42,16 @@ type Envelope struct { | |
Data []byte | ||
Nonce uint64 | ||
|
||
pow float64 // Message-specific PoW as described in the Whisper specification. | ||
hash common.Hash // Cached hash of the envelope to avoid rehashing every time. | ||
// Don't access hash directly, use Hash() function instead. | ||
pow float64 // Message-specific PoW as described in the Whisper specification. | ||
|
||
// the following variables should not be accessed directly, use the corresponding function instead: Hash(), Bloom() | ||
hash common.Hash // Cached hash of the envelope to avoid rehashing every time. | ||
bloom []byte | ||
} | ||
|
||
// size returns the size of envelope as it is sent (i.e. public fields only) | ||
func (e *Envelope) size() int { | ||
const EnvelopeHeaderLength = 20 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't this constant declared in doc.go ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because it is used only once in this function, and never again. we don't need a global constant if it can be local. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree: in spite of momentarily only being used in this specific function, this constant is a characteristic of the envelope format, and therefore its scope is the whole envelope code. So it should definitely be a top-level constant, if not documented in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To press the point further, there are two reasons this should be defined globally:
|
||
return EnvelopeHeaderLength + len(e.Data) | ||
} | ||
|
||
|
@@ -227,3 +230,30 @@ func (e *Envelope) Open(watcher *Filter) (msg *ReceivedMessage) { | |
} | ||
return msg | ||
} | ||
|
||
// Bloom maps 4-bytes Topic into 64-byte bloom filter with 3 bits set (at most). | ||
func (e *Envelope) Bloom() []byte { | ||
if e.bloom == nil { | ||
e.bloom = TopicToBloom(e.Topic) | ||
} | ||
return e.bloom | ||
} | ||
|
||
// TopicToBloom converts the topic (4 bytes) to the bloom filter (64 bytes) | ||
func TopicToBloom(topic TopicType) []byte { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Public functions need a comment |
||
b := make([]byte, bloomFilterSize) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a more descriptive name, maybe? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is good enough. a comment is added as well. |
||
var index [3]int | ||
for j := 0; j < 3; j++ { | ||
index[j] = int(topic[j]) | ||
if (topic[3] & (1 << uint(j))) != 0 { | ||
index[j] += 256 | ||
} | ||
} | ||
|
||
for j := 0; j < 3; j++ { | ||
byteIndex := index[j] / 8 | ||
bitIndex := index[j] % 8 | ||
b[byteIndex] = (1 << uint(bitIndex)) | ||
} | ||
return b | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ type Peer struct { | |
|
||
trusted bool | ||
powRequirement float64 | ||
bloomFilter []byte // may contain nil in case of full node | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of having There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i thought about productivity: most nodes will be full nodes, so comparing 64 bytes in a cycle would be an overkill. but i have to admit that you are absolutely right -- readability (and good engineering practices in gerenral) have higher priority over productivity. however, my first priority today is to deliver the minimum viable product of v6. so, you are welcome to do the necessary refactoring after v6 is realeased. |
||
|
||
known *set.Set // Messages already known by the peer to avoid wasting bandwidth | ||
|
||
|
@@ -74,8 +75,12 @@ func (p *Peer) handshake() error { | |
// Send the handshake status message asynchronously | ||
errc := make(chan error, 1) | ||
go func() { | ||
errc <- p2p.Send(p.ws, statusCode, ProtocolVersion) | ||
pow := p.host.MinPow() | ||
powConverted := math.Float64bits(pow) | ||
bloom := p.host.BloomFilter() | ||
errc <- p2p.SendItems(p.ws, statusCode, ProtocolVersion, powConverted, bloom) | ||
}() | ||
|
||
// Fetch the remote status packet and verify protocol match | ||
packet, err := p.ws.ReadMsg() | ||
if err != nil { | ||
|
@@ -85,14 +90,42 @@ func (p *Peer) handshake() error { | |
return fmt.Errorf("peer [%x] sent packet %x before status packet", p.ID(), packet.Code) | ||
} | ||
s := rlp.NewStream(packet.Payload, uint64(packet.Size)) | ||
peerVersion, err := s.Uint() | ||
_, err = s.List() | ||
if err != nil { | ||
return fmt.Errorf("peer [%x] sent bad status message: %v", p.ID(), err) | ||
} | ||
peerVersion, err := s.Uint() | ||
if err != nil { | ||
return fmt.Errorf("peer [%x] sent bad status message (unable to decode version): %v", p.ID(), err) | ||
} | ||
if peerVersion != ProtocolVersion { | ||
return fmt.Errorf("peer [%x]: protocol version mismatch %d != %d", p.ID(), peerVersion, ProtocolVersion) | ||
} | ||
// Wait until out own status is consumed too | ||
|
||
// only version is mandatory, subsequent parameters are optional | ||
powRaw, err := s.Uint() | ||
if err == nil { | ||
pow := math.Float64frombits(powRaw) | ||
if math.IsInf(pow, 0) || math.IsNaN(pow) || pow < 0.0 { | ||
return fmt.Errorf("peer [%x] sent bad status message: invalid pow", p.ID()) | ||
} | ||
p.powRequirement = pow | ||
|
||
var bloom []byte | ||
err = s.Decode(&bloom) | ||
if err == nil { | ||
sz := len(bloom) | ||
if sz != bloomFilterSize && sz != 0 { | ||
return fmt.Errorf("peer [%x] sent bad status message: wrong bloom filter size %d", p.ID(), sz) | ||
} | ||
if isFullNode(bloom) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. insisting on my previous comment: I think it makes no sense to distinguish between a full and "half" node, it just makes the code harder to read. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i agree, thanx for pointing out! |
||
p.bloomFilter = nil | ||
} else { | ||
p.bloomFilter = bloom | ||
} | ||
} | ||
} | ||
|
||
if err := <-errc; err != nil { | ||
return fmt.Errorf("peer [%x] failed to send status packet: %v", p.ID(), err) | ||
} | ||
|
@@ -156,7 +189,7 @@ func (p *Peer) broadcast() error { | |
envelopes := p.host.Envelopes() | ||
bundle := make([]*Envelope, 0, len(envelopes)) | ||
for _, envelope := range envelopes { | ||
if !p.marked(envelope) && envelope.PoW() >= p.powRequirement { | ||
if !p.marked(envelope) && envelope.PoW() >= p.powRequirement && p.bloomMatch(envelope) { | ||
bundle = append(bundle, envelope) | ||
} | ||
} | ||
|
@@ -186,3 +219,16 @@ func (p *Peer) notifyAboutPowRequirementChange(pow float64) error { | |
i := math.Float64bits(pow) | ||
return p2p.Send(p.ws, powRequirementCode, i) | ||
} | ||
|
||
func (p *Peer) notifyAboutBloomFilterChange(bloom []byte) error { | ||
return p2p.Send(p.ws, bloomFilterExCode, bloom) | ||
} | ||
|
||
func (p *Peer) bloomMatch(env *Envelope) bool { | ||
if p.bloomFilter == nil { | ||
// no filter - full node, accepts all envelops | ||
return true | ||
} | ||
|
||
return bloomFilterMatch(p.bloomFilter, env.Bloom()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as per my previous comment, |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import ( | |
"bytes" | ||
"crypto/ecdsa" | ||
"fmt" | ||
mrand "math/rand" | ||
"net" | ||
"sync" | ||
"testing" | ||
|
@@ -87,6 +88,9 @@ var nodes [NumNodes]*TestNode | |
var sharedKey []byte = []byte("some arbitrary data here") | ||
var sharedTopic TopicType = TopicType{0xF, 0x1, 0x2, 0} | ||
var expectedMessage []byte = []byte("per rectum ad astra") | ||
var masterBloomFilter []byte | ||
var masterPow = 0.00000001 | ||
var round int = 1 | ||
|
||
func TestSimulation(t *testing.T) { | ||
// create a chain of whisper nodes, | ||
|
@@ -104,27 +108,79 @@ func TestSimulation(t *testing.T) { | |
// check if each node have received and decrypted exactly one message | ||
checkPropagation(t, true) | ||
|
||
// send protocol-level messages (powRequirementCode) and check the new PoW requirement values | ||
powReqExchange(t) | ||
// check if Status message was correctly decoded | ||
checkBloomFilterExchange(t) | ||
checkPowExchange(t) | ||
|
||
// send new pow and bloom exchange messages | ||
resetParams(t) | ||
round++ | ||
|
||
// node #1 sends one expected (decryptable) message | ||
sendMsg(t, true, 1) | ||
|
||
// check if each node (except node #0) have received and decrypted exactly one message | ||
checkPropagation(t, false) | ||
|
||
for i := 1; i < NumNodes; i++ { | ||
time.Sleep(20 * time.Millisecond) | ||
sendMsg(t, true, i) | ||
} | ||
|
||
// check if corresponding protocol-level messages were correctly decoded | ||
checkPowExchangeForNodeZero(t) | ||
checkBloomFilterExchange(t) | ||
|
||
stopServers() | ||
} | ||
|
||
func resetParams(t *testing.T) { | ||
// change pow only for node zero | ||
masterPow = 7777777.0 | ||
nodes[0].shh.SetMinimumPoW(masterPow) | ||
|
||
// change bloom for all nodes | ||
masterBloomFilter = TopicToBloom(sharedTopic) | ||
for i := 0; i < NumNodes; i++ { | ||
nodes[i].shh.SetBloomFilter(masterBloomFilter) | ||
} | ||
} | ||
|
||
func initBloom(t *testing.T) { | ||
masterBloomFilter = make([]byte, bloomFilterSize) | ||
_, err := mrand.Read(masterBloomFilter) | ||
if err != nil { | ||
t.Fatalf("rand failed: %s.", err) | ||
} | ||
|
||
msgBloom := TopicToBloom(sharedTopic) | ||
masterBloomFilter = addBloom(masterBloomFilter, msgBloom) | ||
for i := 0; i < 32; i++ { | ||
masterBloomFilter[i] = 0xFF | ||
} | ||
|
||
if !bloomFilterMatch(masterBloomFilter, msgBloom) { | ||
t.Fatalf("bloom mismatch on initBloom.") | ||
} | ||
} | ||
|
||
func initialize(t *testing.T) { | ||
initBloom(t) | ||
|
||
var err error | ||
ip := net.IPv4(127, 0, 0, 1) | ||
port0 := 30303 | ||
|
||
for i := 0; i < NumNodes; i++ { | ||
var node TestNode | ||
b := make([]byte, bloomFilterSize) | ||
copy(b, masterBloomFilter) | ||
node.shh = New(&DefaultConfig) | ||
node.shh.SetMinimumPowTest(0.00000001) | ||
node.shh.SetMinimumPoW(masterPow) | ||
node.shh.SetBloomFilter(b) | ||
if !isBloomFilterEqual(node.shh.BloomFilter(), masterBloomFilter) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use |
||
t.Fatalf("bloom mismatch on init.") | ||
} | ||
node.shh.Start(nil) | ||
topics := make([]TopicType, 0) | ||
topics = append(topics, sharedTopic) | ||
|
@@ -206,7 +262,7 @@ func checkPropagation(t *testing.T, includingNodeZero bool) { | |
for i := first; i < NumNodes; i++ { | ||
f := nodes[i].shh.GetFilter(nodes[i].filerId) | ||
if f == nil { | ||
t.Fatalf("failed to get filterId %s from node %d.", nodes[i].filerId, i) | ||
t.Fatalf("failed to get filterId %s from node %d, round %d.", nodes[i].filerId, i, round) | ||
} | ||
|
||
mail := f.Retrieve() | ||
|
@@ -332,34 +388,43 @@ func TestPeerBasic(t *testing.T) { | |
} | ||
} | ||
|
||
func powReqExchange(t *testing.T) { | ||
func checkPowExchangeForNodeZero(t *testing.T) { | ||
cnt := 0 | ||
for i, node := range nodes { | ||
for peer := range node.shh.peers { | ||
if peer.powRequirement > 1000.0 { | ||
t.Fatalf("node %d: one of the peers' pow requirement is too big (%f).", i, peer.powRequirement) | ||
if peer.peer.ID() == discover.PubkeyID(&nodes[0].id.PublicKey) { | ||
cnt++ | ||
if peer.powRequirement != masterPow { | ||
t.Fatalf("node %d: failed to set the new pow requirement.", i) | ||
} | ||
} | ||
} | ||
} | ||
if cnt == 0 { | ||
t.Fatalf("no matching peers found.") | ||
} | ||
} | ||
|
||
const pow float64 = 7777777.0 | ||
nodes[0].shh.SetMinimumPoW(pow) | ||
|
||
// wait until all the messages are delivered | ||
time.Sleep(64 * time.Millisecond) | ||
|
||
cnt := 0 | ||
func checkPowExchange(t *testing.T) { | ||
for i, node := range nodes { | ||
for peer := range node.shh.peers { | ||
if peer.peer.ID() == discover.PubkeyID(&nodes[0].id.PublicKey) { | ||
cnt++ | ||
if peer.powRequirement != pow { | ||
t.Fatalf("node %d: failed to set the new pow requirement.", i) | ||
if peer.peer.ID() != discover.PubkeyID(&nodes[0].id.PublicKey) { | ||
if peer.powRequirement != masterPow { | ||
t.Fatalf("node %d: failed to exchange pow requirement in round %d; expected %f, got %f", | ||
i, round, masterPow, peer.powRequirement) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
if cnt == 0 { | ||
t.Fatalf("no matching peers found.") | ||
func checkBloomFilterExchange(t *testing.T) { | ||
for i, node := range nodes { | ||
for peer := range node.shh.peers { | ||
if !isBloomFilterEqual(peer.bloomFilter, masterBloomFilter) { | ||
t.Fatalf("node %d: failed to exchange bloom filter requirement in round %d. \n%x expected \n%x got", | ||
i, round, masterBloomFilter, peer.bloomFilter) | ||
} | ||
} | ||
} | ||
} |
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.
can be rewritten:
return (err != nil), err
. Does this need to be part of this changelist, though?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.
my style is easier to read, it's more explicit.