From 0126d014351bc4f58e2d7e6564a054fb80f87153 Mon Sep 17 00:00:00 2001 From: bas-vk Date: Fri, 20 Jan 2017 23:32:16 +0100 Subject: [PATCH 1/9] types: bugfix invalid V derivation on tx json unmarshal (#3594) --- core/types/transaction.go | 3 ++- core/types/transaction_signing.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index e610671d31..0b6d24268b 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -198,7 +198,8 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error { var V byte if isProtectedV((*big.Int)(dec.V)) { - V = byte((new(big.Int).Sub((*big.Int)(dec.V), deriveChainId((*big.Int)(dec.V))).Uint64()) - 35) + chainId := deriveChainId((*big.Int)(dec.V)).Uint64() + V = byte(dec.V.ToInt().Uint64() - 35 - 2*chainId) } else { V = byte(((*big.Int)(dec.V)).Uint64() - 27) } diff --git a/core/types/transaction_signing.go b/core/types/transaction_signing.go index 4ebc789a59..7d7b63e9f1 100644 --- a/core/types/transaction_signing.go +++ b/core/types/transaction_signing.go @@ -160,7 +160,7 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) { // needs to be in the [R || S || V] format where V is 0 or 1. func (s EIP155Signer) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) { if len(sig) != 65 { - panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig))) + panic(fmt.Sprintf("wrong size for signature: got %d, want 65", len(sig))) } cpy := &Transaction{data: tx.data} From 682875adff760a29a2bb0024190883e4b4dd5d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Sat, 21 Jan 2017 00:39:16 +0200 Subject: [PATCH 2/9] accounts/abi/bind, internal/ethapi: binary search gas estimation (#3587) Gas estimation currently mostly works, but can underestimate for more funky refunds. This is because various ops (e.g. CALL) need more gas to run than they actually consume (e.g. 2300 stipend that is refunded if not used). With more intricate contract interplays, it becomes almost impossible to return a proper value to the user. This commit swaps out the simplistic gas estimation to a binary search approach, honing in on the correct gas use. This does mean that gas estimation needs to rerun the transaction log(max-price) times to measure whether it fails or not, but it's a price paid by the transaction issuer, and it should be worth it to support proper estimates. --- accounts/abi/bind/backends/simulated.go | 28 ++++++++++++-- accounts/abi/bind/bind_test.go | 51 +++++++++++++++++++++++-- internal/ethapi/api.go | 30 ++++++++++++++- 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index c26662ea19..1c34ba0e79 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -201,10 +201,32 @@ func (b *SimulatedBackend) SuggestGasPrice(ctx context.Context) (*big.Int, error func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMsg) (*big.Int, error) { b.mu.Lock() defer b.mu.Unlock() - defer b.pendingState.RevertToSnapshot(b.pendingState.Snapshot()) - _, gas, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) - return gas, err + // Binary search the gas requirement, as it may be higher than the amount used + var lo, hi uint64 + if call.Gas != nil { + hi = call.Gas.Uint64() + } else { + hi = b.pendingBlock.GasLimit().Uint64() + } + for lo+1 < hi { + // Take a guess at the gas, and check transaction validity + mid := (hi + lo) / 2 + call.Gas = new(big.Int).SetUint64(mid) + + snapshot := b.pendingState.Snapshot() + _, gas, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) + b.pendingState.RevertToSnapshot(snapshot) + + // If the transaction became invalid or used all the gas (failed), raise the gas limit + if err != nil || gas.Cmp(call.Gas) == 0 { + lo = mid + continue + } + // Otherwise assume the transaction succeeded, lower the gas limit + hi = mid + } + return new(big.Int).SetUint64(hi), nil } // callContract implemens common code between normal and pending contract calls. diff --git a/accounts/abi/bind/bind_test.go b/accounts/abi/bind/bind_test.go index 6ebc8ea0a3..eb46bc081d 100644 --- a/accounts/abi/bind/bind_test.go +++ b/accounts/abi/bind/bind_test.go @@ -341,11 +341,11 @@ var bindTests = []struct { { `NonExistent`, ` - contract NonExistent { - function String() constant returns(string) { - return "I don't exist"; + contract NonExistent { + function String() constant returns(string) { + return "I don't exist"; + } } - } `, `6060604052609f8060106000396000f3606060405260e060020a6000350463f97a60058114601a575b005b600060605260c0604052600d60809081527f4920646f6e27742065786973740000000000000000000000000000000000000060a052602060c0908152600d60e081905281906101009060a09080838184600060046012f15050815172ffffffffffffffffffffffffffffffffffffff1916909152505060405161012081900392509050f3`, `[{"constant":true,"inputs":[],"name":"String","outputs":[{"name":"","type":"string"}],"type":"function"}]`, @@ -365,6 +365,49 @@ var bindTests = []struct { } `, }, + // Tests that gas estimation works for contracts with weird gas mechanics too. + { + `FunkyGasPattern`, + ` + contract FunkyGasPattern { + string public field; + + function SetField(string value) { + // This check will screw gas estimation! Good, good! + if (msg.gas < 100000) { + throw; + } + field = value; + } + } + `, + `606060405261021c806100126000396000f3606060405260e060020a600035046323fcf32a81146100265780634f28bf0e1461007b575b005b6040805160206004803580820135601f8101849004840285018401909552848452610024949193602493909291840191908190840183828082843750949650505050505050620186a05a101561014e57610002565b6100db60008054604080516020601f600260001961010060018816150201909516949094049384018190048102820181019092528281529291908301828280156102145780601f106101e957610100808354040283529160200191610214565b60405180806020018281038252838181518152602001915080519060200190808383829060006004602084601f0104600302600f01f150905090810190601f16801561013b5780820380516001836020036101000a031916815260200191505b509250505060405180910390f35b505050565b8060006000509080519060200190828054600181600116156101000203166002900490600052602060002090601f016020900481019282601f106101b557805160ff19168380011785555b506101499291505b808211156101e557600081556001016101a1565b82800160010185558215610199579182015b828111156101995782518260005055916020019190600101906101c7565b5090565b820191906000526020600020905b8154815290600101906020018083116101f757829003601f168201915b50505050508156`, + `[{"constant":false,"inputs":[{"name":"value","type":"string"}],"name":"SetField","outputs":[],"type":"function"},{"constant":true,"inputs":[],"name":"field","outputs":[{"name":"","type":"string"}],"type":"function"}]`, + ` + // Generate a new random account and a funded simulator + key, _ := crypto.GenerateKey() + auth := bind.NewKeyedTransactor(key) + sim := backends.NewSimulatedBackend(core.GenesisAccount{Address: auth.From, Balance: big.NewInt(10000000000)}) + + // Deploy a funky gas pattern contract + _, _, limiter, err := DeployFunkyGasPattern(auth, sim) + if err != nil { + t.Fatalf("Failed to deploy funky contract: %v", err) + } + sim.Commit() + + // Set the field with automatic estimation and check that it succeeds + auth.GasLimit = nil + if _, err := limiter.SetField(auth, "automatic"); err != nil { + t.Fatalf("Failed to call automatically gased transaction: %v", err) + } + sim.Commit() + + if field, _ := limiter.Field(nil); field != "automatic" { + t.Fatalf("Field mismatch: have %v, want %v", field, "automatic") + } + `, + }, } // Tests that packages generated by the binder can be successfully compiled and diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 4359181c87..925f547b60 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -559,8 +559,34 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNr r // EstimateGas returns an estimate of the amount of gas needed to execute the given transaction. func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args CallArgs) (*hexutil.Big, error) { - _, gas, err := s.doCall(ctx, args, rpc.PendingBlockNumber) - return (*hexutil.Big)(gas), err + // Binary search the gas requirement, as it may be higher than the amount used + var lo, hi uint64 + if (*big.Int)(&args.Gas).BitLen() > 0 { + hi = (*big.Int)(&args.Gas).Uint64() + } else { + // Retrieve the current pending block to act as the gas ceiling + block, err := s.b.BlockByNumber(ctx, rpc.PendingBlockNumber) + if err != nil { + return nil, err + } + hi = block.GasLimit().Uint64() + } + for lo+1 < hi { + // Take a guess at the gas, and check transaction validity + mid := (hi + lo) / 2 + (*big.Int)(&args.Gas).SetUint64(mid) + + _, gas, err := s.doCall(ctx, args, rpc.PendingBlockNumber) + + // If the transaction became invalid or used all the gas (failed), raise the gas limit + if err != nil || gas.Cmp((*big.Int)(&args.Gas)) == 0 { + lo = mid + continue + } + // Otherwise assume the transaction succeeded, lower the gas limit + hi = mid + } + return (*hexutil.Big)(new(big.Int).SetUint64(hi)), nil } // ExecutionResult groups all structured logs emitted by the EVM From 935d891e9db870edde6b37ad84bb21d7df19c383 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sun, 22 Jan 2017 22:14:09 +0100 Subject: [PATCH 3/9] cmd/evm: added debug flag (back) (#3554) * evm: added debug flag (back) * cmd/evm: gofmt --- cmd/evm/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/evm/main.go b/cmd/evm/main.go index 035cf1c545..9f67e66286 100644 --- a/cmd/evm/main.go +++ b/cmd/evm/main.go @@ -161,6 +161,7 @@ func run(ctx *cli.Context) error { Value: common.Big(ctx.GlobalString(ValueFlag.Name)), EVMConfig: vm.Config{ Tracer: logger, + Debug: ctx.GlobalBool(DebugFlag.Name), DisableGasMetering: ctx.GlobalBool(DisableGasMeteringFlag.Name), }, }) @@ -176,6 +177,7 @@ func run(ctx *cli.Context) error { Value: common.Big(ctx.GlobalString(ValueFlag.Name)), EVMConfig: vm.Config{ Tracer: logger, + Debug: ctx.GlobalBool(DebugFlag.Name), DisableGasMetering: ctx.GlobalBool(DisableGasMeteringFlag.Name), }, }) From 96778a1c216f7d0d987dd8ea6474b2d3eebe9cfc Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Sun, 22 Jan 2017 23:28:47 +0100 Subject: [PATCH 4/9] crypto/secp256k1: sign with deterministic K (rfc6979) (#3561) --- crypto/secp256k1/secp256.go | 12 ++++-------- crypto/secp256k1/secp256_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/crypto/secp256k1/secp256.go b/crypto/secp256k1/secp256.go index 070e0d902f..4284115e2d 100644 --- a/crypto/secp256k1/secp256.go +++ b/crypto/secp256k1/secp256.go @@ -40,8 +40,6 @@ import ( "errors" "math/big" "unsafe" - - "github.com/ethereum/go-ethereum/crypto/randentropy" ) var ( @@ -89,13 +87,11 @@ func Sign(msg []byte, seckey []byte) ([]byte, error) { } var ( - msgdata = (*C.uchar)(unsafe.Pointer(&msg[0])) - nonce = randentropy.GetEntropyCSPRNG(32) - noncefunc = &(*C.secp256k1_nonce_function_default) - noncefuncData = unsafe.Pointer(&nonce[0]) - sigstruct C.secp256k1_ecdsa_recoverable_signature + msgdata = (*C.uchar)(unsafe.Pointer(&msg[0])) + noncefunc = C.secp256k1_nonce_function_rfc6979 + sigstruct C.secp256k1_ecdsa_recoverable_signature ) - if C.secp256k1_ecdsa_sign_recoverable(context, &sigstruct, msgdata, seckeydata, noncefunc, noncefuncData) == 0 { + if C.secp256k1_ecdsa_sign_recoverable(context, &sigstruct, msgdata, seckeydata, noncefunc, nil) == 0 { return nil, ErrSignFailed } diff --git a/crypto/secp256k1/secp256_test.go b/crypto/secp256k1/secp256_test.go index ec28b8e39e..287ab512ec 100644 --- a/crypto/secp256k1/secp256_test.go +++ b/crypto/secp256k1/secp256_test.go @@ -112,6 +112,24 @@ func TestSignAndRecover(t *testing.T) { } } +func TestSignDeterministic(t *testing.T) { + _, seckey := generateKeyPair() + msg := make([]byte, 32) + copy(msg, "hi there") + + sig1, err := Sign(msg, seckey) + if err != nil { + t.Fatal(err) + } + sig2, err := Sign(msg, seckey) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(sig1, sig2) { + t.Fatal("signatures not equal") + } +} + func TestRandomMessagesWithSameKey(t *testing.T) { pubkey, seckey := generateKeyPair() keys := func() ([]byte, []byte) { From c04598f2b09ab66204b30b1c9242b4dfadcd6bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 23 Jan 2017 11:46:15 +0200 Subject: [PATCH 5/9] containers/docker: update to alpine 3.5, add CA certificates --- containers/docker/develop-alpine/Dockerfile | 4 ++-- containers/docker/master-alpine/Dockerfile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/containers/docker/develop-alpine/Dockerfile b/containers/docker/develop-alpine/Dockerfile index 3393c43374..a8d85bc63a 100644 --- a/containers/docker/develop-alpine/Dockerfile +++ b/containers/docker/develop-alpine/Dockerfile @@ -1,7 +1,7 @@ -FROM alpine:3.4 +FROM alpine:3.5 RUN \ - apk add --update go git make gcc musl-dev && \ + apk add --update go git make gcc musl-dev ca-certificates && \ git clone --depth 1 https://github.com/ethereum/go-ethereum && \ (cd go-ethereum && make geth) && \ cp go-ethereum/build/bin/geth /geth && \ diff --git a/containers/docker/master-alpine/Dockerfile b/containers/docker/master-alpine/Dockerfile index 5131c473a6..0db583a439 100644 --- a/containers/docker/master-alpine/Dockerfile +++ b/containers/docker/master-alpine/Dockerfile @@ -1,7 +1,7 @@ -FROM alpine:3.4 +FROM alpine:3.5 RUN \ - apk add --update go git make gcc musl-dev && \ + apk add --update go git make gcc musl-dev ca-certificates && \ git clone --depth 1 --branch release/1.5 https://github.com/ethereum/go-ethereum && \ (cd go-ethereum && make geth) && \ cp go-ethereum/build/bin/geth /geth && \ From 0b9070fe019a79740af3271109c6ce550911cb60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 23 Jan 2017 12:12:38 +0200 Subject: [PATCH 6/9] containers/docker: update ubuntu images to build, not pull --- containers/docker/develop-ubuntu/Dockerfile | 22 ++++++++++----------- containers/docker/master-ubuntu/Dockerfile | 22 ++++++++++----------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/containers/docker/develop-ubuntu/Dockerfile b/containers/docker/develop-ubuntu/Dockerfile index 98b4aadf8f..c79becb55a 100644 --- a/containers/docker/develop-ubuntu/Dockerfile +++ b/containers/docker/develop-ubuntu/Dockerfile @@ -1,17 +1,15 @@ -FROM ubuntu:wily -MAINTAINER caktux +FROM ubuntu:xenial -ENV DEBIAN_FRONTEND noninteractive - -RUN apt-get update && \ - apt-get upgrade -q -y && \ - apt-get dist-upgrade -q -y && \ - apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 923F6CA9 && \ - echo "deb http://ppa.launchpad.net/ethereum/ethereum-dev/ubuntu wily main" | tee -a /etc/apt/sources.list.d/ethereum.list && \ - apt-get update && \ - apt-get install -q -y geth +RUN \ + apt-get update && apt-get upgrade -q -y && \ + apt-get install -y --no-install-recommends golang git make gcc libc-dev ca-certificates && \ + git clone --depth 1 https://github.com/ethereum/go-ethereum && \ + (cd go-ethereum && make geth) && \ + cp go-ethereum/build/bin/geth /geth && \ + apt-get remove -y golang git make gcc libc-dev && apt autoremove -y && apt-get clean && \ + rm -rf /go-ethereum EXPOSE 8545 EXPOSE 30303 -ENTRYPOINT ["/usr/bin/geth"] +ENTRYPOINT ["/geth"] diff --git a/containers/docker/master-ubuntu/Dockerfile b/containers/docker/master-ubuntu/Dockerfile index 2c6de28c92..877ae94e99 100644 --- a/containers/docker/master-ubuntu/Dockerfile +++ b/containers/docker/master-ubuntu/Dockerfile @@ -1,17 +1,15 @@ -FROM ubuntu:wily -MAINTAINER caktux +FROM ubuntu:xenial -ENV DEBIAN_FRONTEND noninteractive - -RUN apt-get update && \ - apt-get upgrade -q -y && \ - apt-get dist-upgrade -q -y && \ - apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 923F6CA9 && \ - echo "deb http://ppa.launchpad.net/ethereum/ethereum/ubuntu wily main" | tee -a /etc/apt/sources.list.d/ethereum.list && \ - apt-get update && \ - apt-get install -q -y geth +RUN \ + apt-get update && apt-get upgrade -q -y && \ + apt-get install -y --no-install-recommends golang git make gcc libc-dev ca-certificates && \ + git clone --depth 1 --branch release/1.5 https://github.com/ethereum/go-ethereum && \ + (cd go-ethereum && make geth) && \ + cp go-ethereum/build/bin/geth /geth && \ + apt-get remove -y golang git make gcc libc-dev && apt autoremove -y && apt-get clean && \ + rm -rf /go-ethereum EXPOSE 8545 EXPOSE 30303 -ENTRYPOINT ["/usr/bin/geth"] +ENTRYPOINT ["/geth"] From fc52f2c007fb4a9065cd1b0ec402f7cbbae4dc5e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 23 Jan 2017 18:51:02 +0100 Subject: [PATCH 7/9] core/types: make Transaction zero value printable (#3595) --- core/types/transaction.go | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 0b6d24268b..9382acb70b 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -134,7 +134,7 @@ func (tx *Transaction) ChainId() *big.Int { return deriveChainId(tx.data.V) } -// Protected returns whether the transaction is pretected from replay protection +// Protected returns whether the transaction is protected from replay protection. func (tx *Transaction) Protected() bool { return isProtectedV(tx.data.V) } @@ -311,16 +311,20 @@ func (tx *Transaction) RawSignatureValues() (*big.Int, *big.Int, *big.Int) { } func (tx *Transaction) String() string { - // make a best guess about the signer and use that to derive - // the sender. - signer := deriveSigner(tx.data.V) - var from, to string - if f, err := Sender(signer, tx); err != nil { // derive but don't cache - from = "[invalid sender: invalid sig]" + if tx.data.V != nil { + // make a best guess about the signer and use that to derive + // the sender. + signer := deriveSigner(tx.data.V) + if f, err := Sender(signer, tx); err != nil { // derive but don't cache + from = "[invalid sender: invalid sig]" + } else { + from = fmt.Sprintf("%x", f[:]) + } } else { - from = fmt.Sprintf("%x", f[:]) + from = "[invalid sender: nil V field]" } + if tx.data.Recipient == nil { to = "[contract creation]" } else { @@ -333,13 +337,13 @@ func (tx *Transaction) String() string { From: %s To: %s Nonce: %v - GasPrice: %v - GasLimit %v - Value: %v + GasPrice: %#x + GasLimit %#x + Value: %#x Data: 0x%x - V: 0x%x - R: 0x%x - S: 0x%x + V: %#x + R: %#x + S: %#x Hex: %x `, tx.Hash(), From f1069a30b9ca13769ace154ff25dfabc83f25485 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 24 Jan 2017 13:20:37 +0100 Subject: [PATCH 8/9] eth/downloader: improve deliverNodeData (#3588) Commit d3b751e accidentally deleted a crucial 'return' statement, leading to a crash in case of an issue with node data. This change improves the fix in PR #3591 by removing the lock entirely. --- eth/downloader/queue.go | 124 +++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 72 deletions(-) diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 3318879e28..5be09f37dc 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "sync" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -101,10 +100,9 @@ type queue struct { stateTaskQueue *prque.Prque // [eth/63] Priority queue of the hashes to fetch the node data for statePendPool map[string]*fetchRequest // [eth/63] Currently pending node data retrieval operations - stateDatabase ethdb.Database // [eth/63] Trie database to populate during state reassembly - stateScheduler *state.StateSync // [eth/63] State trie synchronisation scheduler and integrator - stateProcessors int32 // [eth/63] Number of currently running state processors - stateSchedLock sync.RWMutex // [eth/63] Lock serialising access to the state scheduler + stateDatabase ethdb.Database // [eth/63] Trie database to populate during state reassembly + stateScheduler *state.StateSync // [eth/63] State trie synchronisation scheduler and integrator + stateWriters int // [eth/63] Number of running state DB writer goroutines resultCache []*fetchResult // Downloaded but not yet delivered fetch results resultOffset uint64 // Offset of the first cached fetch result in the block chain @@ -143,9 +141,6 @@ func (q *queue) Reset() { q.lock.Lock() defer q.lock.Unlock() - q.stateSchedLock.Lock() - defer q.stateSchedLock.Unlock() - q.closed = false q.mode = FullSync q.fastSyncPivot = 0 @@ -209,13 +204,24 @@ func (q *queue) PendingReceipts() int { // PendingNodeData retrieves the number of node data entries pending for retrieval. func (q *queue) PendingNodeData() int { - q.stateSchedLock.RLock() - defer q.stateSchedLock.RUnlock() + q.lock.Lock() + defer q.lock.Unlock() + return q.pendingNodeDataLocked() +} + +// pendingNodeDataLocked retrieves the number of node data entries pending for retrieval. +// The caller must hold q.lock. +func (q *queue) pendingNodeDataLocked() int { + var n int if q.stateScheduler != nil { - return q.stateScheduler.Pending() + n = q.stateScheduler.Pending() } - return 0 + // Ensure that PendingNodeData doesn't return 0 until all state is written. + if q.stateWriters > 0 { + n++ + } + return n } // InFlightHeaders retrieves whether there are header fetch requests currently @@ -251,7 +257,7 @@ func (q *queue) InFlightNodeData() bool { q.lock.Lock() defer q.lock.Unlock() - return len(q.statePendPool)+int(atomic.LoadInt32(&q.stateProcessors)) > 0 + return len(q.statePendPool)+q.stateWriters > 0 } // Idle returns if the queue is fully idle or has some data still inside. This @@ -264,12 +270,9 @@ func (q *queue) Idle() bool { pending := len(q.blockPendPool) + len(q.receiptPendPool) + len(q.statePendPool) cached := len(q.blockDonePool) + len(q.receiptDonePool) - q.stateSchedLock.RLock() if q.stateScheduler != nil { queued += q.stateScheduler.Pending() } - q.stateSchedLock.RUnlock() - return (queued + pending + cached) == 0 } @@ -398,9 +401,7 @@ func (q *queue) Schedule(headers []*types.Header, from uint64) []*types.Header { req.Hashes = make(map[common.Hash]int) // Make sure executing requests fail, but don't disappear } - q.stateSchedLock.Lock() q.stateScheduler = state.NewStateSync(header.Root, q.stateDatabase) - q.stateSchedLock.Unlock() } inserts = append(inserts, header) q.headerHead = hash @@ -459,7 +460,7 @@ func (q *queue) countProcessableItems() int { // resultCache has space for fsHeaderForceVerify items. Not // doing this could leave us unable to download the required // amount of headers. - if i > 0 || len(q.stateTaskPool) > 0 || q.PendingNodeData() > 0 { + if i > 0 || len(q.stateTaskPool) > 0 || q.pendingNodeDataLocked() > 0 { return i } for j := 0; j < fsHeaderForceVerify; j++ { @@ -524,9 +525,6 @@ func (q *queue) ReserveHeaders(p *peer, count int) *fetchRequest { func (q *queue) ReserveNodeData(p *peer, count int) *fetchRequest { // Create a task generator to fetch status-fetch tasks if all schedules ones are done generator := func(max int) { - q.stateSchedLock.Lock() - defer q.stateSchedLock.Unlock() - if q.stateScheduler != nil { for _, hash := range q.stateScheduler.Missing(max) { q.stateTaskPool[hash] = q.stateTaskIndex @@ -1068,7 +1066,7 @@ func (q *queue) DeliverNodeData(id string, data [][]byte, callback func(int, boo } } // Iterate over the downloaded data and verify each of them - accepted, errs := 0, make([]error, 0) + errs := make([]error, 0) process := []trie.SyncResult{} for _, blob := range data { // Skip any state trie entries that were not requested @@ -1079,70 +1077,52 @@ func (q *queue) DeliverNodeData(id string, data [][]byte, callback func(int, boo } // Inject the next state trie item into the processing queue process = append(process, trie.SyncResult{Hash: hash, Data: blob}) - accepted++ - delete(request.Hashes, hash) delete(q.stateTaskPool, hash) } - // Start the asynchronous node state data injection - atomic.AddInt32(&q.stateProcessors, 1) - go func() { - defer atomic.AddInt32(&q.stateProcessors, -1) - q.deliverNodeData(process, callback) - }() // Return all failed or missing fetches to the queue for hash, index := range request.Hashes { q.stateTaskQueue.Push(hash, float32(index)) } + if q.stateScheduler == nil { + return 0, errNoFetchesPending + } + + // Run valid nodes through the trie download scheduler. It writes completed nodes to a + // batch, which is committed asynchronously. This may lead to over-fetches because the + // scheduler treats everything as written after Process has returned, but it's + // unlikely to be an issue in practice. + batch := q.stateDatabase.NewBatch() + progressed, nproc, procerr := q.stateScheduler.Process(process, batch) + q.stateWriters += 1 + go func() { + if procerr == nil { + nproc = len(process) + procerr = batch.Write() + } + // Return processing errors through the callback so the sync gets canceled. The + // number of writers is decremented prior to the call so PendingNodeData will + // return zero when the callback runs. + q.lock.Lock() + q.stateWriters -= 1 + q.lock.Unlock() + callback(nproc, progressed, procerr) + // Wake up WaitResults after the state has been written because it might be + // waiting for completion of the pivot block's state download. + q.active.Signal() + }() + // If none of the data items were good, it's a stale delivery switch { case len(errs) == 0: - return accepted, nil + return len(process), nil case len(errs) == len(request.Hashes): - return accepted, errStaleDelivery + return len(process), errStaleDelivery default: - return accepted, fmt.Errorf("multiple failures: %v", errs) + return len(process), fmt.Errorf("multiple failures: %v", errs) } } -// deliverNodeData is the asynchronous node data processor that injects a batch -// of sync results into the state scheduler. -func (q *queue) deliverNodeData(results []trie.SyncResult, callback func(int, bool, error)) { - // Wake up WaitResults after the state has been written because it - // might be waiting for the pivot block state to get completed. - defer q.active.Signal() - - // Process results one by one to permit task fetches in between - progressed := false - for i, result := range results { - q.stateSchedLock.Lock() - - if q.stateScheduler == nil { - // Syncing aborted since this async delivery started, bail out - q.stateSchedLock.Unlock() - callback(i, progressed, errNoFetchesPending) - return - } - - batch := q.stateDatabase.NewBatch() - prog, _, err := q.stateScheduler.Process([]trie.SyncResult{result}, batch) - if err != nil { - q.stateSchedLock.Unlock() - callback(i, progressed, err) - return - } - if err = batch.Write(); err != nil { - q.stateSchedLock.Unlock() - callback(i, progressed, err) - return // TODO(karalabe): If a DB write fails (disk full), we ought to cancel the sync - } - // Item processing succeeded, release the lock (temporarily) - progressed = progressed || prog - q.stateSchedLock.Unlock() - } - callback(len(results), progressed, nil) -} - // Prepare configures the result cache to allow accepting and caching inbound // fetch results. func (q *queue) Prepare(offset uint64, mode SyncMode, pivot uint64, head *types.Header) { From 1886d03faa9b7d8cdf335da84c297d30c213bb69 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 25 Jan 2017 16:29:40 +0100 Subject: [PATCH 9/9] console, internal/web3ext: remove bzz and ens extensions (#3602) web3.js includes bzz methods and throws an error when the extension module is reregistered. The ENS RPC API is deprecated and not exposed by anything. --- console/console.go | 4 ++ internal/web3ext/web3ext.go | 97 ------------------------------------- 2 files changed, 4 insertions(+), 97 deletions(-) diff --git a/console/console.go b/console/console.go index 9bb3df9265..389d528580 100644 --- a/console/console.go +++ b/console/console.go @@ -137,10 +137,14 @@ func (c *Console) init(preload []string) error { continue // manually mapped or ignore } if file, ok := web3ext.Modules[api]; ok { + // Load our extension for the module. if err = c.jsre.Compile(fmt.Sprintf("%s.js", api), file); err != nil { return fmt.Errorf("%s.js: %v", api, err) } flatten += fmt.Sprintf("var %s = web3.%s; ", api, api) + } else if obj, err := c.jsre.Run("web3." + api); err == nil && obj.IsObject() { + // Enable web3.js built-in extension if available. + flatten += fmt.Sprintf("var %s = web3.%s; ", api, api) } } if _, err = c.jsre.Run(flatten); err != nil { diff --git a/internal/web3ext/web3ext.go b/internal/web3ext/web3ext.go index e10e660450..edbe45fa34 100644 --- a/internal/web3ext/web3ext.go +++ b/internal/web3ext/web3ext.go @@ -19,10 +19,8 @@ package web3ext var Modules = map[string]string{ "admin": Admin_JS, - "bzz": Bzz_JS, "chequebook": Chequebook_JS, "debug": Debug_JS, - "ens": ENS_JS, "eth": Eth_JS, "miner": Miner_JS, "net": Net_JS, @@ -32,101 +30,6 @@ var Modules = map[string]string{ "txpool": TxPool_JS, } -const Bzz_JS = ` -web3._extend({ - property: 'bzz', - methods: - [ - new web3._extend.Method({ - name: 'syncEnabled', - call: 'bzz_syncEnabled', - params: 1, - inputFormatter: [null] - }), - new web3._extend.Method({ - name: 'swapEnabled', - call: 'bzz_swapEnabled', - params: 1, - inputFormatter: [null] - }), - new web3._extend.Method({ - name: 'download', - call: 'bzz_download', - params: 2, - inputFormatter: [null, null] - }), - new web3._extend.Method({ - name: 'upload', - call: 'bzz_upload', - params: 2, - inputFormatter: [null, null] - }), - new web3._extend.Method({ - name: 'resolve', - call: 'bzz_resolve', - params: 1, - inputFormatter: [null] - }), - new web3._extend.Method({ - name: 'get', - call: 'bzz_get', - params: 1, - inputFormatter: [null] - }), - new web3._extend.Method({ - name: 'put', - call: 'bzz_put', - params: 2, - inputFormatter: [null, null] - }), - new web3._extend.Method({ - name: 'modify', - call: 'bzz_modify', - params: 4, - inputFormatter: [null, null, null, null] - }) - ], - properties: - [ - new web3._extend.Property({ - name: 'hive', - getter: 'bzz_hive' - }), - new web3._extend.Property({ - name: 'info', - getter: 'bzz_info', - }), - ] -}); -` - -const ENS_JS = ` -web3._extend({ - property: 'ens', - methods: - [ - new web3._extend.Method({ - name: 'register', - call: 'ens_register', - params: 1, - inputFormatter: [null] - }), - new web3._extend.Method({ - name: 'setContentHash', - call: 'ens_setContentHash', - params: 2, - inputFormatter: [null, null] - }), - new web3._extend.Method({ - name: 'resolve', - call: 'ens_resolve', - params: 1, - inputFormatter: [null] - }), - ] -}) -` - const Chequebook_JS = ` web3._extend({ property: 'chequebook',