From 0df317dbeb19c08d835e93299ac3ceb29b0c577e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 12 Aug 2015 13:46:36 +0300 Subject: [PATCH] eth: upgrade the block header query packet to new spec --- eth/handler.go | 68 +++++++++++++++----- eth/handler_test.go | 150 +++++++++++++++++++++++++++++-------------- eth/protocol.go | 47 ++++++++++++++ eth/protocol_test.go | 45 +++++++++++++ 4 files changed, 247 insertions(+), 63 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index 689d6d5ae2..25ff0eef03 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -336,29 +336,67 @@ func (pm *ProtocolManager) handleMsg(p *peer) error { pm.downloader.DeliverBlocks(p.id, blocks) } + // Block header query, collect the requested headers and reply case p.version >= eth62 && msg.Code == GetBlockHeadersMsg: - // Decode the retrieval message - msgStream := rlp.NewStream(msg.Payload, uint64(msg.Size)) - if _, err := msgStream.List(); err != nil { - return err + // Decode the complex header query + var query getBlockHeadersData + if err := msg.Decode(&query); err != nil { + return errResp(ErrDecode, "%v: %v", msg, err) } // Gather blocks until the fetch or network limits is reached var ( - hash common.Hash bytes common.StorageSize headers []*types.Header + unknown bool ) - for bytes < softResponseLimit && len(headers) < downloader.MaxHeaderFetch { - //Retrieve the hash of the next block - if err := msgStream.Decode(&hash); err == rlp.EOL { - break - } else if err != nil { - return errResp(ErrDecode, "msg %v: %v", msg, err) + for !unknown && len(headers) < int(query.Amount) && bytes < softResponseLimit && len(headers) < downloader.MaxHeaderFetch { + // Retrieve the next block satisfying the query + var origin *types.Block + if query.Origin.Hash != (common.Hash{}) { + origin = pm.chainman.GetBlock(query.Origin.Hash) + } else { + origin = pm.chainman.GetBlockByNumber(query.Origin.Number) } - // Retrieve the requested block, stopping if enough was found - if block := pm.chainman.GetBlock(hash); block != nil { - headers = append(headers, block.Header()) - bytes += block.Size() + if origin == nil { + break + } + headers = append(headers, origin.Header()) + bytes += origin.Size() + + // Advance to the next block of the query + switch { + case query.Origin.Hash != (common.Hash{}) && query.Reverse: + // Hash based traversal towards the genesis block + for i := 0; i < int(query.Skip)+1; i++ { + if block := pm.chainman.GetBlock(query.Origin.Hash); block != nil { + query.Origin.Hash = block.ParentHash() + } else { + unknown = true + break + } + } + case query.Origin.Hash != (common.Hash{}) && !query.Reverse: + // Hash based traversal towards the leaf block + if block := pm.chainman.GetBlockByNumber(origin.NumberU64() + query.Skip + 1); block != nil { + if pm.chainman.GetBlockHashesFromHash(block.Hash(), query.Skip+1)[query.Skip] == query.Origin.Hash { + query.Origin.Hash = block.Hash() + } else { + unknown = true + } + } else { + unknown = true + } + case query.Reverse: + // Number based traversal towards the genesis block + if query.Origin.Number >= query.Skip+1 { + query.Origin.Number -= (query.Skip + 1) + } else { + unknown = true + } + + case !query.Reverse: + // Number based traversal towards the leaf block + query.Origin.Number += (query.Skip + 1) } } return p.SendBlockHeaders(headers) diff --git a/eth/handler_test.go b/eth/handler_test.go index bde44a22c7..63c94faa12 100644 --- a/eth/handler_test.go +++ b/eth/handler_test.go @@ -174,7 +174,7 @@ func testGetBlocks(t *testing.T, protocol int) { } } -// Tests that block headers can be retrieved from a remote chain based on their hashes. +// Tests that block headers can be retrieved from a remote chain based on user queries. func TestGetBlockHeaders62(t *testing.T) { testGetBlockHeaders(t, 62) } func TestGetBlockHeaders63(t *testing.T) { testGetBlockHeaders(t, 63) } func TestGetBlockHeaders64(t *testing.T) { testGetBlockHeaders(t, 64) } @@ -184,62 +184,116 @@ func testGetBlockHeaders(t *testing.T, protocol int) { peer, _ := newTestPeer("peer", protocol, pm, true) defer peer.close() + // Create a "random" unknown hash for testing + var unknown common.Hash + for i, _ := range unknown { + unknown[i] = byte(i) + } // Create a batch of tests for various scenarios - limit := downloader.MaxHeaderFetch + limit := uint64(downloader.MaxHeaderFetch) tests := []struct { - random int // Number of blocks to fetch randomly from the chain - explicit []common.Hash // Explicitly requested blocks - available []bool // Availability of explicitly requested blocks - expected int // Total number of existing blocks to expect + query *getBlockHeadersData // The query to execute for header retrieval + expect []common.Hash // The hashes of the block whose headers are expected }{ - {1, nil, nil, 1}, // A single random block should be retrievable - {10, nil, nil, 10}, // Multiple random blocks should be retrievable - {limit, nil, nil, limit}, // The maximum possible blocks should be retrievable - {limit + 1, nil, nil, limit}, // No more that the possible block count should be returned - {0, []common.Hash{pm.chainman.Genesis().Hash()}, []bool{true}, 1}, // The genesis block should be retrievable - {0, []common.Hash{pm.chainman.CurrentBlock().Hash()}, []bool{true}, 1}, // The chains head block should be retrievable - {0, []common.Hash{common.Hash{}}, []bool{false}, 0}, // A non existent block should not be returned - - // Existing and non-existing blocks interleaved should not cause problems - {0, []common.Hash{ - common.Hash{}, - pm.chainman.GetBlockByNumber(1).Hash(), - common.Hash{}, - pm.chainman.GetBlockByNumber(10).Hash(), - common.Hash{}, - pm.chainman.GetBlockByNumber(100).Hash(), - common.Hash{}, - }, []bool{false, true, false, true, false, true, false}, 3}, + // A single random block should be retrievable by hash and number too + { + &getBlockHeadersData{Origin: hashOrNumber{Hash: pm.chainman.GetBlockByNumber(limit / 2).Hash()}, Amount: 1}, + []common.Hash{pm.chainman.GetBlockByNumber(limit / 2).Hash()}, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: limit / 2}, Amount: 1}, + []common.Hash{pm.chainman.GetBlockByNumber(limit / 2).Hash()}, + }, + // Multiple headers should be retrievable in both directions + { + &getBlockHeadersData{Origin: hashOrNumber{Number: limit / 2}, Amount: 3}, + []common.Hash{ + pm.chainman.GetBlockByNumber(limit / 2).Hash(), + pm.chainman.GetBlockByNumber(limit/2 + 1).Hash(), + pm.chainman.GetBlockByNumber(limit/2 + 2).Hash(), + }, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: limit / 2}, Amount: 3, Reverse: true}, + []common.Hash{ + pm.chainman.GetBlockByNumber(limit / 2).Hash(), + pm.chainman.GetBlockByNumber(limit/2 - 1).Hash(), + pm.chainman.GetBlockByNumber(limit/2 - 2).Hash(), + }, + }, + // Multiple headers with skip lists should be retrievable + { + &getBlockHeadersData{Origin: hashOrNumber{Number: limit / 2}, Skip: 3, Amount: 3}, + []common.Hash{ + pm.chainman.GetBlockByNumber(limit / 2).Hash(), + pm.chainman.GetBlockByNumber(limit/2 + 4).Hash(), + pm.chainman.GetBlockByNumber(limit/2 + 8).Hash(), + }, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: limit / 2}, Skip: 3, Amount: 3, Reverse: true}, + []common.Hash{ + pm.chainman.GetBlockByNumber(limit / 2).Hash(), + pm.chainman.GetBlockByNumber(limit/2 - 4).Hash(), + pm.chainman.GetBlockByNumber(limit/2 - 8).Hash(), + }, + }, + // The chain endpoints should be retrievable + { + &getBlockHeadersData{Origin: hashOrNumber{Number: 0}, Amount: 1}, + []common.Hash{pm.chainman.GetBlockByNumber(0).Hash()}, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: pm.chainman.CurrentBlock().NumberU64()}, Amount: 1}, + []common.Hash{pm.chainman.CurrentBlock().Hash()}, + }, + // Ensure protocol limits are honored + { + &getBlockHeadersData{Origin: hashOrNumber{Number: pm.chainman.CurrentBlock().NumberU64() - 1}, Amount: limit + 10, Reverse: true}, + pm.chainman.GetBlockHashesFromHash(pm.chainman.CurrentBlock().Hash(), limit), + }, + // Check that requesting more than available is handled gracefully + { + &getBlockHeadersData{Origin: hashOrNumber{Number: pm.chainman.CurrentBlock().NumberU64() - 4}, Skip: 3, Amount: 3}, + []common.Hash{ + pm.chainman.GetBlockByNumber(pm.chainman.CurrentBlock().NumberU64() - 4).Hash(), + pm.chainman.GetBlockByNumber(pm.chainman.CurrentBlock().NumberU64()).Hash(), + }, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: 4}, Skip: 3, Amount: 3, Reverse: true}, + []common.Hash{ + pm.chainman.GetBlockByNumber(4).Hash(), + pm.chainman.GetBlockByNumber(0).Hash(), + }, + }, + // Check that requesting more than available is handled gracefully, even if mid skip + { + &getBlockHeadersData{Origin: hashOrNumber{Number: pm.chainman.CurrentBlock().NumberU64() - 4}, Skip: 2, Amount: 3}, + []common.Hash{ + pm.chainman.GetBlockByNumber(pm.chainman.CurrentBlock().NumberU64() - 4).Hash(), + pm.chainman.GetBlockByNumber(pm.chainman.CurrentBlock().NumberU64() - 1).Hash(), + }, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: 4}, Skip: 2, Amount: 3, Reverse: true}, + []common.Hash{ + pm.chainman.GetBlockByNumber(4).Hash(), + pm.chainman.GetBlockByNumber(1).Hash(), + }, + }, + // Check that non existing headers aren't returned + { + &getBlockHeadersData{Origin: hashOrNumber{Hash: unknown}, Amount: 1}, + []common.Hash{}, + }, { + &getBlockHeadersData{Origin: hashOrNumber{Number: pm.chainman.CurrentBlock().NumberU64() + 1}, Amount: 1}, + []common.Hash{}, + }, } // Run each of the tests and verify the results against the chain for i, tt := range tests { - // Collect the hashes to request, and the response to expect - hashes, seen := []common.Hash{}, make(map[int64]bool) + // Collect the headers to expect in the response headers := []*types.Header{} - - for j := 0; j < tt.random; j++ { - for { - num := rand.Int63n(int64(pm.chainman.CurrentBlock().NumberU64())) - if !seen[num] { - seen[num] = true - - block := pm.chainman.GetBlockByNumber(uint64(num)) - hashes = append(hashes, block.Hash()) - if len(headers) < tt.expected { - headers = append(headers, block.Header()) - } - break - } - } - } - for j, hash := range tt.explicit { - hashes = append(hashes, hash) - if tt.available[j] && len(headers) < tt.expected { - headers = append(headers, pm.chainman.GetBlock(hash).Header()) - } + for _, hash := range tt.expect { + headers = append(headers, pm.chainman.GetBlock(hash).Header()) } // Send the hash request and verify the response - p2p.Send(peer.app, 0x03, hashes) + p2p.Send(peer.app, 0x03, tt.query) if err := p2p.ExpectMsg(peer.app, 0x04, headers); err != nil { t.Errorf("test %d: headers mismatch: %v", i, err) } diff --git a/eth/protocol.go b/eth/protocol.go index 8baabef27f..8bed8d22ec 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -17,10 +17,13 @@ package eth import ( + "fmt" + "io" "math/big" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" ) // Constants to match up protocol versions and messages @@ -153,6 +156,50 @@ type getBlockHashesFromNumberData struct { Amount uint64 } +// getBlockHeadersData represents a block header query. +type getBlockHeadersData struct { + Origin hashOrNumber // Block from which to retrieve headers + Amount uint64 // Maximum number of headers to retrieve + Skip uint64 // Blocks to skip between consecutive headers + Reverse bool // Query direction (false = rising towards latest, true = falling towards genesis) +} + +// hashOrNumber is a combined field for specifying an origin block. +type hashOrNumber struct { + Hash common.Hash // Block hash from which to retrieve headers (excludes Number) + Number uint64 // Block hash from which to retrieve headers (excludes Hash) +} + +// EncodeRLP is a specialized encoder for hashOrNumber to encode only one of the +// two contained union fields. +func (hn *hashOrNumber) EncodeRLP(w io.Writer) error { + if hn.Hash == (common.Hash{}) { + return rlp.Encode(w, hn.Number) + } + if hn.Number != 0 { + return fmt.Errorf("both origin hash (%x) and number (%d) provided", hn.Hash, hn.Number) + } + return rlp.Encode(w, hn.Hash) +} + +// DecodeRLP is a specialized decoder for hashOrNumber to decode the contents +// into either a block hash or a block number. +func (hn *hashOrNumber) DecodeRLP(s *rlp.Stream) error { + _, size, _ := s.Kind() + origin, err := s.Raw() + if err == nil { + switch { + case size == 32: + err = rlp.DecodeBytes(origin, &hn.Hash) + case size <= 8: + err = rlp.DecodeBytes(origin, &hn.Number) + default: + err = fmt.Errorf("invalid input size %d for origin", size) + } + } + return err +} + // newBlockData is the network packet for the block propagation message. type newBlockData struct { Block *types.Block diff --git a/eth/protocol_test.go b/eth/protocol_test.go index 32d08ca587..263088099d 100644 --- a/eth/protocol_test.go +++ b/eth/protocol_test.go @@ -27,6 +27,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/p2p" + "github.com/ethereum/go-ethereum/rlp" ) func init() { @@ -181,3 +182,47 @@ func testSendTransactions(t *testing.T, protocol int) { } wg.Wait() } + +// Tests that the custom union field encoder and decoder works correctly. +func TestGetBlockHeadersDataEncodeDecode(t *testing.T) { + // Create a "random" hash for testing + var hash common.Hash + for i, _ := range hash { + hash[i] = byte(i) + } + // Assemble some table driven tests + tests := []struct { + packet *getBlockHeadersData + fail bool + }{ + // Providing the origin as either a hash or a number should both work + {fail: false, packet: &getBlockHeadersData{Origin: hashOrNumber{Number: 314}}}, + {fail: false, packet: &getBlockHeadersData{Origin: hashOrNumber{Hash: hash}}}, + + // Providing arbitrary query field should also work + {fail: false, packet: &getBlockHeadersData{Origin: hashOrNumber{Number: 314}, Amount: 314, Skip: 1, Reverse: true}}, + {fail: false, packet: &getBlockHeadersData{Origin: hashOrNumber{Hash: hash}, Amount: 314, Skip: 1, Reverse: true}}, + + // Providing both the origin hash and origin number must fail + {fail: true, packet: &getBlockHeadersData{Origin: hashOrNumber{Hash: hash, Number: 314}}}, + } + // Iterate over each of the tests and try to encode and then decode + for i, tt := range tests { + bytes, err := rlp.EncodeToBytes(tt.packet) + if err != nil && !tt.fail { + t.Fatalf("test %d: failed to encode packet: %v", i, err) + } else if err == nil && tt.fail { + t.Fatalf("test %d: encode should have failed", i) + } + if !tt.fail { + packet := new(getBlockHeadersData) + if err := rlp.DecodeBytes(bytes, packet); err != nil { + t.Fatalf("test %d: failed to decode packet: %v", i, err) + } + if packet.Origin.Hash != tt.packet.Origin.Hash || packet.Origin.Number != tt.packet.Origin.Number || packet.Amount != tt.packet.Amount || + packet.Skip != tt.packet.Skip || packet.Reverse != tt.packet.Reverse { + t.Fatalf("test %d: encode decode mismatch: have %+v, want %+v", i, packet, tt.packet) + } + } + } +}