Skip to content

Commit bdf3dd1

Browse files
karalabekaralabe
committed
eth: various nitpick fixes on eth/68
1 parent 3678d0d commit bdf3dd1

File tree

10 files changed

+55
-25
lines changed

10 files changed

+55
-25
lines changed

‎cmd/devp2p/internal/ethtest/types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func (msg NewBlock) Code() int { return 23 }
127127
func (msg NewBlock) ReqID() uint64 { return 0 }
128128

129129
// NewPooledTransactionHashes is the network packet for the tx hash propagation message.
130-
type NewPooledTransactionHashes eth.NewPooledTransactionHashesPacket67
130+
type NewPooledTransactionHashes eth.NewPooledTransactionHashesPacket66
131131

132132
func (msg NewPooledTransactionHashes) Code() int { return 24 }
133133
func (msg NewPooledTransactionHashes) ReqID() uint64 { return 0 }

‎eth/handler_eth.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (h *ethHandler) Handle(peer *eth.Peer, packet eth.Packet) error {
6767
case *eth.NewBlockPacket:
6868
return h.handleBlockBroadcast(peer, packet.Block, packet.TD)
6969

70-
case *eth.NewPooledTransactionHashesPacket67:
70+
case *eth.NewPooledTransactionHashesPacket66:
7171
return h.txFetcher.Notify(peer.ID(), *packet)
7272

7373
case *eth.NewPooledTransactionHashesPacket68:

‎eth/handler_eth_test.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (h *testEthHandler) Handle(peer *eth.Peer, packet eth.Packet) error {
6161
h.blockBroadcasts.Send(packet.Block)
6262
return nil
6363

64-
case *eth.NewPooledTransactionHashesPacket67:
64+
case *eth.NewPooledTransactionHashesPacket66:
6565
h.txAnnounces.Send(([]common.Hash)(*packet))
6666
return nil
6767

@@ -85,6 +85,8 @@ func (h *testEthHandler) Handle(peer *eth.Peer, packet eth.Packet) error {
8585
// Tests that peers are correctly accepted (or rejected) based on the advertised
8686
// fork IDs in the protocol handshake.
8787
func TestForkIDSplit66(t *testing.T) { testForkIDSplit(t, eth.ETH66) }
88+
func TestForkIDSplit67(t *testing.T) { testForkIDSplit(t, eth.ETH67) }
89+
func TestForkIDSplit68(t *testing.T) { testForkIDSplit(t, eth.ETH68) }
8890

8991
func testForkIDSplit(t *testing.T, protocol uint) {
9092
t.Parallel()
@@ -239,6 +241,8 @@ func testForkIDSplit(t *testing.T, protocol uint) {
239241

240242
// Tests that received transactions are added to the local pool.
241243
func TestRecvTransactions66(t *testing.T) { testRecvTransactions(t, eth.ETH66) }
244+
func TestRecvTransactions67(t *testing.T) { testRecvTransactions(t, eth.ETH67) }
245+
func TestRecvTransactions68(t *testing.T) { testRecvTransactions(t, eth.ETH68) }
242246

243247
func testRecvTransactions(t *testing.T, protocol uint) {
244248
t.Parallel()
@@ -296,6 +300,7 @@ func testRecvTransactions(t *testing.T, protocol uint) {
296300

297301
// This test checks that pending transactions are sent.
298302
func TestSendTransactions66(t *testing.T) { testSendTransactions(t, eth.ETH66) }
303+
func TestSendTransactions67(t *testing.T) { testSendTransactions(t, eth.ETH67) }
299304
func TestSendTransactions68(t *testing.T) { testSendTransactions(t, eth.ETH68) }
300305

301306
func testSendTransactions(t *testing.T, protocol uint) {
@@ -355,7 +360,7 @@ func testSendTransactions(t *testing.T, protocol uint) {
355360
seen := make(map[common.Hash]struct{})
356361
for len(seen) < len(insert) {
357362
switch protocol {
358-
case 66, 68:
363+
case 66, 67, 68:
359364
select {
360365
case hashes := <-anns:
361366
for _, hash := range hashes {
@@ -382,7 +387,7 @@ func testSendTransactions(t *testing.T, protocol uint) {
382387
// Tests that transactions get propagated to all attached peers, either via direct
383388
// broadcasts or via announcements/retrievals.
384389
func TestTransactionPropagation66(t *testing.T) { testTransactionPropagation(t, eth.ETH66) }
385-
390+
func TestTransactionPropagation67(t *testing.T) { testTransactionPropagation(t, eth.ETH67) }
386391
func TestTransactionPropagation68(t *testing.T) { testTransactionPropagation(t, eth.ETH68) }
387392

388393
func testTransactionPropagation(t *testing.T, protocol uint) {
@@ -685,6 +690,8 @@ func testBroadcastBlock(t *testing.T, peers, bcasts int) {
685690
// Tests that a propagated malformed block (uncles or transactions don't match
686691
// with the hashes in the header) gets discarded and not broadcast forward.
687692
func TestBroadcastMalformedBlock66(t *testing.T) { testBroadcastMalformedBlock(t, eth.ETH66) }
693+
func TestBroadcastMalformedBlock67(t *testing.T) { testBroadcastMalformedBlock(t, eth.ETH67) }
694+
func TestBroadcastMalformedBlock68(t *testing.T) { testBroadcastMalformedBlock(t, eth.ETH68) }
688695

689696
func testBroadcastMalformedBlock(t *testing.T, protocol uint) {
690697
t.Parallel()

‎eth/protocols/eth/broadcast.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,16 @@ func (p *Peer) announceTransactions() {
164164
done = make(chan struct{})
165165
go func() {
166166
if p.version >= ETH68 {
167-
if err := p.sendPooledTransactions(pending, pendingTypes, pendingSizes); err != nil {
167+
if err := p.sendPooledTransactionHashes68(pending, pendingTypes, pendingSizes); err != nil {
168168
fail <- err
169169
return
170170
}
171171
} else {
172-
if err := p.sendPooledTransactionHashes(pending); err != nil {
172+
if err := p.sendPooledTransactionHashes66(pending); err != nil {
173173
fail <- err
174174
return
175175
}
176176
}
177-
178177
close(done)
179178
p.Log().Trace("Sent transaction announcements", "count", len(pending))
180179
}()

‎eth/protocols/eth/handler.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ var eth66 = map[uint64]msgHandler{
168168
NewBlockHashesMsg: handleNewBlockhashes,
169169
NewBlockMsg: handleNewBlock,
170170
TransactionsMsg: handleTransactions,
171-
NewPooledTransactionHashesMsg: handleNewPooledTransactionHashes,
171+
NewPooledTransactionHashesMsg: handleNewPooledTransactionHashes66,
172172
GetBlockHeadersMsg: handleGetBlockHeaders66,
173173
BlockHeadersMsg: handleBlockHeaders66,
174174
GetBlockBodiesMsg: handleGetBlockBodies66,
@@ -185,7 +185,7 @@ var eth67 = map[uint64]msgHandler{
185185
NewBlockHashesMsg: handleNewBlockhashes,
186186
NewBlockMsg: handleNewBlock,
187187
TransactionsMsg: handleTransactions,
188-
NewPooledTransactionHashesMsg: handleNewPooledTransactionHashes,
188+
NewPooledTransactionHashesMsg: handleNewPooledTransactionHashes66,
189189
GetBlockHeadersMsg: handleGetBlockHeaders66,
190190
BlockHeadersMsg: handleBlockHeaders66,
191191
GetBlockBodiesMsg: handleGetBlockBodies66,

‎eth/protocols/eth/handler_test.go‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ func (b *testBackend) Handle(*Peer, Packet) error {
112112

113113
// Tests that block headers can be retrieved from a remote chain based on user queries.
114114
func TestGetBlockHeaders66(t *testing.T) { testGetBlockHeaders(t, ETH66) }
115+
func TestGetBlockHeaders67(t *testing.T) { testGetBlockHeaders(t, ETH67) }
116+
func TestGetBlockHeaders68(t *testing.T) { testGetBlockHeaders(t, ETH68) }
115117

116118
func testGetBlockHeaders(t *testing.T, protocol uint) {
117119
t.Parallel()
@@ -297,6 +299,8 @@ func testGetBlockHeaders(t *testing.T, protocol uint) {
297299

298300
// Tests that block contents can be retrieved from a remote chain based on their hashes.
299301
func TestGetBlockBodies66(t *testing.T) { testGetBlockBodies(t, ETH66) }
302+
func TestGetBlockBodies67(t *testing.T) { testGetBlockBodies(t, ETH67) }
303+
func TestGetBlockBodies68(t *testing.T) { testGetBlockBodies(t, ETH68) }
300304

301305
func testGetBlockBodies(t *testing.T, protocol uint) {
302306
t.Parallel()
@@ -379,9 +383,11 @@ func testGetBlockBodies(t *testing.T, protocol uint) {
379383
}
380384

381385
// Tests that the state trie nodes can be retrieved based on hashes.
382-
func TestGetNodeData66(t *testing.T) { testGetNodeData(t, ETH66) }
386+
func TestGetNodeData66(t *testing.T) { testGetNodeData(t, ETH66, false) }
387+
func TestGetNodeData67(t *testing.T) { testGetNodeData(t, ETH67, true) }
388+
func TestGetNodeData68(t *testing.T) { testGetNodeData(t, ETH68, true) }
383389

384-
func testGetNodeData(t *testing.T, protocol uint) {
390+
func testGetNodeData(t *testing.T, protocol uint, drop bool) {
385391
t.Parallel()
386392

387393
// Define three accounts to simulate transactions with
@@ -442,8 +448,15 @@ func testGetNodeData(t *testing.T, protocol uint) {
442448
GetNodeDataPacket: hashes,
443449
})
444450
msg, err := peer.app.ReadMsg()
445-
if err != nil {
446-
t.Fatalf("failed to read node data response: %v", err)
451+
if !drop {
452+
if err != nil {
453+
t.Fatalf("failed to read node data response: %v", err)
454+
}
455+
} else {
456+
if err != nil {
457+
return
458+
}
459+
t.Fatalf("succeeded to read node data response on non-supporting protocol: %v", msg)
447460
}
448461
if msg.Code != NodeDataMsg {
449462
t.Fatalf("response packet code mismatch: have %x, want %x", msg.Code, NodeDataMsg)
@@ -489,6 +502,8 @@ func testGetNodeData(t *testing.T, protocol uint) {
489502

490503
// Tests that the transaction receipts can be retrieved based on hashes.
491504
func TestGetBlockReceipts66(t *testing.T) { testGetBlockReceipts(t, ETH66) }
505+
func TestGetBlockReceipts67(t *testing.T) { testGetBlockReceipts(t, ETH67) }
506+
func TestGetBlockReceipts68(t *testing.T) { testGetBlockReceipts(t, ETH68) }
492507

493508
func testGetBlockReceipts(t *testing.T, protocol uint) {
494509
t.Parallel()

‎eth/protocols/eth/handlers.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,13 +430,13 @@ func handleReceipts66(backend Backend, msg Decoder, peer *Peer) error {
430430
}, metadata)
431431
}
432432

433-
func handleNewPooledTransactionHashes(backend Backend, msg Decoder, peer *Peer) error {
433+
func handleNewPooledTransactionHashes66(backend Backend, msg Decoder, peer *Peer) error {
434434
// New transaction announcement arrived, make sure we have
435435
// a valid and fresh chain to handle them
436436
if !backend.AcceptTxs() {
437437
return nil
438438
}
439-
ann := new(NewPooledTransactionHashesPacket67)
439+
ann := new(NewPooledTransactionHashesPacket66)
440440
if err := msg.Decode(ann); err != nil {
441441
return fmt.Errorf("%w: message %v: %v", errDecode, msg, err)
442442
}

‎eth/protocols/eth/peer.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,19 +210,26 @@ func (p *Peer) AsyncSendTransactions(hashes []common.Hash) {
210210
}
211211
}
212212

213-
// sendPooledTransactionHashes sends transaction hashes to the peer and includes
213+
// sendPooledTransactionHashes66 sends transaction hashes to the peer and includes
214214
// them in its transaction hash set for future reference.
215215
//
216216
// This method is a helper used by the async transaction announcer. Don't call it
217217
// directly as the queueing (memory) and transmission (bandwidth) costs should
218218
// not be managed directly.
219-
func (p *Peer) sendPooledTransactionHashes(hashes []common.Hash) error {
219+
func (p *Peer) sendPooledTransactionHashes66(hashes []common.Hash) error {
220220
// Mark all the transactions as known, but ensure we don't overflow our limits
221221
p.knownTxs.Add(hashes...)
222-
return p2p.Send(p.rw, NewPooledTransactionHashesMsg, NewPooledTransactionHashesPacket67(hashes))
222+
return p2p.Send(p.rw, NewPooledTransactionHashesMsg, NewPooledTransactionHashesPacket66(hashes))
223223
}
224224

225-
func (p *Peer) sendPooledTransactions(hashes []common.Hash, types []byte, sizes []uint32) error {
225+
// sendPooledTransactionHashes68 sends transaction hashes (tagged with their type
226+
// and size) to the peer and includes them in its transaction hash set for future
227+
// reference.
228+
//
229+
// This method is a helper used by the async transaction announcer. Don't call it
230+
// directly as the queueing (memory) and transmission (bandwidth) costs should
231+
// not be managed directly.
232+
func (p *Peer) sendPooledTransactionHashes68(hashes []common.Hash, types []byte, sizes []uint32) error {
226233
// Mark all the transactions as known, but ensure we don't overflow our limits
227234
p.knownTxs.Add(hashes...)
228235
return p2p.Send(p.rw, NewPooledTransactionHashesMsg, NewPooledTransactionHashesPacket68{Types: types, Sizes: sizes, Hashes: hashes})

‎eth/protocols/eth/peer_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ func newTestPeer(name string, version uint, backend Backend) (*testPeer, <-chan
4848
peer := NewPeer(version, p2p.NewPeer(id, name, nil), net, backend.TxPool())
4949
errc := make(chan error, 1)
5050
go func() {
51+
defer app.Close()
52+
5153
errc <- backend.RunPeer(peer, func(peer *Peer) error {
5254
return Handle(backend, peer)
5355
})

‎eth/protocols/eth/protocol.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,10 @@ type ReceiptsRLPPacket66 struct {
299299
ReceiptsRLPPacket
300300
}
301301

302-
// NewPooledTransactionHashesPacket67 represents a transaction announcement packet.
303-
type NewPooledTransactionHashesPacket67 []common.Hash
302+
// NewPooledTransactionHashesPacket66 represents a transaction announcement packet on eth/66 and eth/67.
303+
type NewPooledTransactionHashesPacket66 []common.Hash
304304

305-
// NewPooledTransactionHashesPacket68 represents a transaction announcement packet over eth/68.
305+
// NewPooledTransactionHashesPacket68 represents a transaction announcement packet on eth/68 and newer.
306306
type NewPooledTransactionHashesPacket68 struct {
307307
Types []byte
308308
Sizes []uint32
@@ -372,8 +372,8 @@ func (*GetReceiptsPacket) Kind() byte { return GetReceiptsMsg }
372372
func (*ReceiptsPacket) Name() string { return "Receipts" }
373373
func (*ReceiptsPacket) Kind() byte { return ReceiptsMsg }
374374

375-
func (*NewPooledTransactionHashesPacket67) Name() string { return "NewPooledTransactionHashes" }
376-
func (*NewPooledTransactionHashesPacket67) Kind() byte { return NewPooledTransactionHashesMsg }
375+
func (*NewPooledTransactionHashesPacket66) Name() string { return "NewPooledTransactionHashes" }
376+
func (*NewPooledTransactionHashesPacket66) Kind() byte { return NewPooledTransactionHashesMsg }
377377

378378
func (*NewPooledTransactionHashesPacket68) Name() string { return "NewPooledTransactionHashes" }
379379
func (*NewPooledTransactionHashesPacket68) Kind() byte { return NewPooledTransactionHashesMsg }

0 commit comments

Comments
 (0)