From 65e9874da314da4f5a72115639a86551e7768bbf Mon Sep 17 00:00:00 2001 From: Ian Norden Date: Fri, 17 May 2019 10:20:28 -0500 Subject: [PATCH] review fixes and fixes for issues ran into in integration --- cmd/geth/main.go | 2 +- cmd/geth/usage.go | 2 +- cmd/utils/flags.go | 12 ++++++------ statediff/api.go | 14 ++++++++------ statediff/service.go | 12 ++++++------ 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 755100168..8d4e9f9e8 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -150,7 +150,7 @@ var ( utils.EVMInterpreterFlag, utils.StateDiffFlag, utils.StateDiffPathsAndProofs, - utils.StateDiffAllNodeTypes, + utils.StateDiffIntermediateNodes, utils.StateDiffStreamBlock, utils.StateDiffWatchedAddresses, configFileFlag, diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index d6664d5c9..49bb0d32d 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -267,7 +267,7 @@ var AppHelpFlagGroups = []flagGroup{ Flags: []cli.Flag{ utils.StateDiffFlag, utils.StateDiffPathsAndProofs, - utils.StateDiffAllNodeTypes, + utils.StateDiffIntermediateNodes, utils.StateDiffWatchedAddresses, utils.StateDiffStreamBlock, }, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index b3c0b356a..677e82f7d 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -765,11 +765,11 @@ var ( } StateDiffPathsAndProofs = cli.BoolFlag{ Name: "statediff.pathsandproofs", - Usage: "Set to true to generate paths and proof sets for diffed state and storage trie lead nodes", + Usage: "Set to true to generate paths and proof sets for diffed state and storage trie leaf nodes", } - StateDiffAllNodeTypes = cli.BoolFlag{ - Name: "statediff.allnodes", - Usage: "Set to true to consider all node types: leaf, branch, and extension; default (false) processes leaf nodes only", + StateDiffIntermediateNodes = cli.BoolFlag{ + Name: "statediff.intermediatenodes", + Usage: "Set to include intermediate (branch and extension) nodes; default (false) processes leaf nodes only", } StateDiffWatchedAddresses = cli.StringSliceFlag{ Name: "statediff.watchedaddresses", @@ -777,7 +777,7 @@ var ( } StateDiffStreamBlock = cli.BoolFlag{ Name: "statediff.streamblock", - Usage: "Set to true to stream the block data alongside state diff data", + Usage: "Set to true to stream the block data alongside state diff data in the same subscription payload", } ) @@ -1642,7 +1642,7 @@ func RegisterStateDiffService(stack *node.Node, ctx *cli.Context) { config := statediff.Config{ StreamBlock: ctx.GlobalBool(StateDiffStreamBlock.Name), PathsAndProofs: ctx.GlobalBool(StateDiffPathsAndProofs.Name), - AllNodes: ctx.GlobalBool(StateDiffAllNodeTypes.Name), + AllNodes: ctx.GlobalBool(StateDiffIntermediateNodes.Name), WatchedAddresses: ctx.GlobalStringSlice(StateDiffWatchedAddresses.Name), } if err := stack.Register(func(ctx *node.ServiceContext) (node.Service, error) { diff --git a/statediff/api.go b/statediff/api.go index 498c2f759..75f590daf 100644 --- a/statediff/api.go +++ b/statediff/api.go @@ -44,7 +44,7 @@ func NewPublicStateDiffAPI(sds IService) *PublicStateDiffAPI { } // Subscribe is the public method to setup a subscription that fires off state-diff payloads as they are created -func (api *PublicStateDiffAPI) Subscribe(ctx context.Context) (*rpc.Subscription, error) { +func (api *PublicStateDiffAPI) Subscribe(ctx context.Context, payloadChan chan Payload) (*rpc.Subscription, error) { // ensure that the RPC connection supports subscriptions notifier, supported := rpc.NotifierFromContext(ctx) if !supported { @@ -56,19 +56,21 @@ func (api *PublicStateDiffAPI) Subscribe(ctx context.Context) (*rpc.Subscription go func() { // subscribe to events from the state diff service - payloadChannel := make(chan Payload) + payloadChannel := make(chan Payload, 10) quitChan := make(chan bool) api.sds.Subscribe(rpcSub.ID, payloadChannel, quitChan) - - // loop and await state diff payloads and relay them to the subscriber with then notifier + // loop and await state diff payloads and relay them to the subscriber with the notifier for { select { case packet := <-payloadChannel: if err := notifier.Notify(rpcSub.ID, packet); err != nil { log.Error("Failed to send state diff packet", "err", err) } - case <-rpcSub.Err(): - err := api.sds.Unsubscribe(rpcSub.ID) + case err := <-rpcSub.Err(): + log.Error("State diff service rpcSub error", err) + println("err") + println(err.Error()) + err = api.sds.Unsubscribe(rpcSub.ID) if err != nil { log.Error("Failed to unsubscribe from the state diff service", err) } diff --git a/statediff/service.go b/statediff/service.go index 5e3f3e59f..49cebef5f 100644 --- a/statediff/service.go +++ b/statediff/service.go @@ -129,11 +129,11 @@ func (sds *Service) Loop(chainEventCh chan core.ChainEvent) { log.Error("Error building statediff", "block number", currentBlock.Number(), "error", err) } case err := <-errCh: - log.Warn("Error from chain event subscription, breaking loop.", "error", err) + log.Warn("Error from chain event subscription, breaking loop", "error", err) sds.close() return case <-sds.QuitChan: - log.Info("Quitting the statediff block channel") + log.Info("Quitting the statediffing process") sds.close() return } @@ -214,9 +214,9 @@ func (sds *Service) send(payload Payload) { for id, sub := range sds.Subscriptions { select { case sub.PayloadChan <- payload: - log.Info("sending state diff payload to subscription %s", id) + log.Info(fmt.Sprintf("sending state diff payload to subscription %s", id)) default: - log.Info("unable to send payload to subscription %s; channel has no receiver", id) + log.Info(fmt.Sprintf("unable to send payload to subscription %s", id)) } } sds.Unlock() @@ -229,9 +229,9 @@ func (sds *Service) close() { select { case sub.QuitChan <- true: delete(sds.Subscriptions, id) - log.Info("closing subscription %s", id) + log.Info(fmt.Sprintf("closing subscription %s", id)) default: - log.Info("unable to close subscription %s; channel has no receiver", id) + log.Info(fmt.Sprintf("unable to close subscription %s; channel has no receiver", id)) } } sds.Unlock()