From 18d000f625f69688d19dc1c68a6447e9d0cb4ec7 Mon Sep 17 00:00:00 2001 From: D-Stacks <78099568+D-Stacks@users.noreply.github.com> Date: Wed, 29 Jun 2022 14:54:40 +0200 Subject: [PATCH] Logger: change log level for "Couldn't find UTXO entry" to debug (and ignore message on orphan transactions) (#2108) * Logger: change log level for "Couldn't find UTXO entry" to debug * ignore error for orphans * continue also if orphans * check if blocks exist * safe rpc mode * limit window size * verify block status depending on context * allow a 2 factor gap in expected mergeset size Co-authored-by: msutton --- .../v5/blockrelay/handle_ibd_block_locator.go | 5 ++-- .../blockrelay/handle_ibd_block_requests.go | 3 +-- .../blockrelay/handle_relay_block_requests.go | 3 +-- .../v5/blockrelay/handle_request_anticone.go | 4 ++-- .../v5/blockrelay/handle_request_headers.go | 24 ++++++++++++++++--- app/rpc/rpchandlers/add_peer.go | 8 +++++++ app/rpc/rpchandlers/ban.go | 8 +++++++ .../estimate_network_hashes_per_second.go | 21 ++++++++++++++++ .../get_mempool_entries_by_addresses.go | 12 ++++++---- .../rpchandlers/resolve_finality_conflict.go | 8 +++++++ app/rpc/rpchandlers/shut_down.go | 8 +++++++ app/rpc/rpchandlers/unban.go | 8 +++++++ .../consensus/model/externalapi/blockinfo.go | 10 ++++++++ infrastructure/config/config.go | 1 + 14 files changed, 107 insertions(+), 16 deletions(-) diff --git a/app/protocol/flows/v5/blockrelay/handle_ibd_block_locator.go b/app/protocol/flows/v5/blockrelay/handle_ibd_block_locator.go index 4e9cd0bc4..250c5cf9b 100644 --- a/app/protocol/flows/v5/blockrelay/handle_ibd_block_locator.go +++ b/app/protocol/flows/v5/blockrelay/handle_ibd_block_locator.go @@ -5,7 +5,6 @@ import ( "github.com/kaspanet/kaspad/app/protocol/peer" "github.com/kaspanet/kaspad/app/protocol/protocolerrors" "github.com/kaspanet/kaspad/domain" - "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" "github.com/kaspanet/kaspad/infrastructure/network/netadapter/router" ) @@ -34,7 +33,7 @@ func HandleIBDBlockLocator(context HandleIBDBlockLocatorContext, incomingRoute * if err != nil { return err } - if !blockInfo.Exists { + if !blockInfo.HasHeader() { return protocolerrors.Errorf(true, "received IBDBlockLocator "+ "with an unknown targetHash %s", targetHash) } @@ -47,7 +46,7 @@ func HandleIBDBlockLocator(context HandleIBDBlockLocatorContext, incomingRoute * } // The IBD block locator is checking only existing blocks with bodies. - if !blockInfo.Exists || blockInfo.BlockStatus == externalapi.StatusHeaderOnly { + if !blockInfo.HasBody() { continue } diff --git a/app/protocol/flows/v5/blockrelay/handle_ibd_block_requests.go b/app/protocol/flows/v5/blockrelay/handle_ibd_block_requests.go index fa6348581..aa69b82a0 100644 --- a/app/protocol/flows/v5/blockrelay/handle_ibd_block_requests.go +++ b/app/protocol/flows/v5/blockrelay/handle_ibd_block_requests.go @@ -4,7 +4,6 @@ import ( "github.com/kaspanet/kaspad/app/appmessage" "github.com/kaspanet/kaspad/app/protocol/protocolerrors" "github.com/kaspanet/kaspad/domain" - "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" "github.com/kaspanet/kaspad/infrastructure/network/netadapter/router" "github.com/pkg/errors" ) @@ -32,7 +31,7 @@ func HandleIBDBlockRequests(context HandleIBDBlockRequestsContext, incomingRoute if err != nil { return err } - if !blockInfo.Exists || blockInfo.BlockStatus == externalapi.StatusHeaderOnly { + if !blockInfo.HasBody() { return protocolerrors.Errorf(true, "block %s not found (v5)", hash) } block, err := context.Domain().Consensus().GetBlock(hash) diff --git a/app/protocol/flows/v5/blockrelay/handle_relay_block_requests.go b/app/protocol/flows/v5/blockrelay/handle_relay_block_requests.go index db4e54558..552f5f103 100644 --- a/app/protocol/flows/v5/blockrelay/handle_relay_block_requests.go +++ b/app/protocol/flows/v5/blockrelay/handle_relay_block_requests.go @@ -5,7 +5,6 @@ import ( peerpkg "github.com/kaspanet/kaspad/app/protocol/peer" "github.com/kaspanet/kaspad/app/protocol/protocolerrors" "github.com/kaspanet/kaspad/domain" - "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" "github.com/kaspanet/kaspad/infrastructure/network/netadapter/router" "github.com/pkg/errors" ) @@ -33,7 +32,7 @@ func HandleRelayBlockRequests(context RelayBlockRequestsContext, incomingRoute * if err != nil { return err } - if !blockInfo.Exists || blockInfo.BlockStatus == externalapi.StatusHeaderOnly { + if !blockInfo.HasBody() { return protocolerrors.Errorf(true, "block %s not found", hash) } block, err := context.Domain().Consensus().GetBlock(hash) diff --git a/app/protocol/flows/v5/blockrelay/handle_request_anticone.go b/app/protocol/flows/v5/blockrelay/handle_request_anticone.go index 267204953..208af8c6b 100644 --- a/app/protocol/flows/v5/blockrelay/handle_request_anticone.go +++ b/app/protocol/flows/v5/blockrelay/handle_request_anticone.go @@ -47,9 +47,9 @@ func (flow *handleRequestAnticoneFlow) start() error { // GetAnticone is expected to be called by the syncee for getting the anticone of the header selected tip // intersected by past of relayed block, and is thus expected to be bounded by mergeset limit since - // we relay blocks only if they enter virtual's mergeset. We add 2 for a small margin error. + // we relay blocks only if they enter virtual's mergeset. We add a 2 factor for possible sync gaps. blockHashes, err := flow.Domain().Consensus().GetAnticone(blockHash, contextHash, - flow.Config().ActiveNetParams.MergeSetSizeLimit+2) + flow.Config().ActiveNetParams.MergeSetSizeLimit*2) if err != nil { return protocolerrors.Wrap(true, err, "Failed querying anticone") } diff --git a/app/protocol/flows/v5/blockrelay/handle_request_headers.go b/app/protocol/flows/v5/blockrelay/handle_request_headers.go index c85fdc5cb..38225efb0 100644 --- a/app/protocol/flows/v5/blockrelay/handle_request_headers.go +++ b/app/protocol/flows/v5/blockrelay/handle_request_headers.go @@ -46,7 +46,25 @@ func (flow *handleRequestHeadersFlow) start() error { } log.Debugf("Received requestHeaders with lowHash: %s, highHash: %s", lowHash, highHash) - isLowSelectedAncestorOfHigh, err := flow.Domain().Consensus().IsInSelectedParentChainOf(lowHash, highHash) + consensus := flow.Domain().Consensus() + + lowHashInfo, err := consensus.GetBlockInfo(lowHash) + if err != nil { + return err + } + if !lowHashInfo.HasHeader() { + return protocolerrors.Errorf(true, "Block %s does not exist", lowHash) + } + + highHashInfo, err := consensus.GetBlockInfo(highHash) + if err != nil { + return err + } + if !highHashInfo.HasHeader() { + return protocolerrors.Errorf(true, "Block %s does not exist", highHash) + } + + isLowSelectedAncestorOfHigh, err := consensus.IsInSelectedParentChainOf(lowHash, highHash) if err != nil { return err } @@ -62,7 +80,7 @@ func (flow *handleRequestHeadersFlow) start() error { // in order to avoid locking the consensus for too long // maxBlocks MUST be >= MergeSetSizeLimit + 1 const maxBlocks = 1 << 10 - blockHashes, _, err := flow.Domain().Consensus().GetHashesBetween(lowHash, highHash, maxBlocks) + blockHashes, _, err := consensus.GetHashesBetween(lowHash, highHash, maxBlocks) if err != nil { return err } @@ -70,7 +88,7 @@ func (flow *handleRequestHeadersFlow) start() error { blockHeaders := make([]*appmessage.MsgBlockHeader, len(blockHashes)) for i, blockHash := range blockHashes { - blockHeader, err := flow.Domain().Consensus().GetBlockHeader(blockHash) + blockHeader, err := consensus.GetBlockHeader(blockHash) if err != nil { return err } diff --git a/app/rpc/rpchandlers/add_peer.go b/app/rpc/rpchandlers/add_peer.go index 7b6c83122..8563ebe42 100644 --- a/app/rpc/rpchandlers/add_peer.go +++ b/app/rpc/rpchandlers/add_peer.go @@ -9,6 +9,14 @@ import ( // HandleAddPeer handles the respectively named RPC command func HandleAddPeer(context *rpccontext.Context, _ *router.Router, request appmessage.Message) (appmessage.Message, error) { + if context.Config.SafeRPC { + log.Warn("AddPeer RPC command called while node in safe RPC mode -- ignoring.") + response := appmessage.NewAddPeerResponseMessage() + response.Error = + appmessage.RPCErrorf("AddPeer RPC command called while node in safe RPC mode") + return response, nil + } + AddPeerRequest := request.(*appmessage.AddPeerRequestMessage) address, err := network.NormalizeAddress(AddPeerRequest.Address, context.Config.ActiveNetParams.DefaultPort) if err != nil { diff --git a/app/rpc/rpchandlers/ban.go b/app/rpc/rpchandlers/ban.go index 0a96b53e0..0d47d01b3 100644 --- a/app/rpc/rpchandlers/ban.go +++ b/app/rpc/rpchandlers/ban.go @@ -9,6 +9,14 @@ import ( // HandleBan handles the respectively named RPC command func HandleBan(context *rpccontext.Context, _ *router.Router, request appmessage.Message) (appmessage.Message, error) { + if context.Config.SafeRPC { + log.Warn("Ban RPC command called while node in safe RPC mode -- ignoring.") + response := appmessage.NewBanResponseMessage() + response.Error = + appmessage.RPCErrorf("Ban RPC command called while node in safe RPC mode") + return response, nil + } + banRequest := request.(*appmessage.BanRequestMessage) ip := net.ParseIP(banRequest.IP) if ip == nil { diff --git a/app/rpc/rpchandlers/estimate_network_hashes_per_second.go b/app/rpc/rpchandlers/estimate_network_hashes_per_second.go index 505c62b2a..3e760bdd0 100644 --- a/app/rpc/rpchandlers/estimate_network_hashes_per_second.go +++ b/app/rpc/rpchandlers/estimate_network_hashes_per_second.go @@ -27,6 +27,27 @@ func HandleEstimateNetworkHashesPerSecond( } } + if context.Config.SafeRPC { + const windowSizeLimit = 10000 + if windowSize > windowSizeLimit { + response := &appmessage.EstimateNetworkHashesPerSecondResponseMessage{} + response.Error = + appmessage.RPCErrorf( + "Requested window size %d is larger than max allowed in RPC safe mode (%d)", + windowSize, windowSizeLimit) + return response, nil + } + } + + if uint64(windowSize) > context.Config.ActiveNetParams.PruningDepth() { + response := &appmessage.EstimateNetworkHashesPerSecondResponseMessage{} + response.Error = + appmessage.RPCErrorf( + "Requested window size %d is larger than pruning point depth %d", + windowSize, context.Config.ActiveNetParams.PruningDepth()) + return response, nil + } + networkHashesPerSecond, err := context.Domain.Consensus().EstimateNetworkHashesPerSecond(startHash, windowSize) if err != nil { response := &appmessage.EstimateNetworkHashesPerSecondResponseMessage{} diff --git a/app/rpc/rpchandlers/get_mempool_entries_by_addresses.go b/app/rpc/rpchandlers/get_mempool_entries_by_addresses.go index d40d42332..04e8ac30b 100644 --- a/app/rpc/rpchandlers/get_mempool_entries_by_addresses.go +++ b/app/rpc/rpchandlers/get_mempool_entries_by_addresses.go @@ -43,7 +43,7 @@ func HandleGetMempoolEntriesByAddresses(context *rpccontext.Context, _ *router.R if getMempoolEntriesByAddressesRequest.IncludeOrphanPool { orphanPoolTransactions := context.Domain.MiningManager().AllOrphanTransactions() - orphanPoolEntriesByAddresse, err := extractMempoolEntriesByAddressesFromTransactions( + orphanPoolEntriesByAddress, err := extractMempoolEntriesByAddressesFromTransactions( context, getMempoolEntriesByAddressesRequest.Addresses, orphanPoolTransactions, @@ -59,7 +59,7 @@ func HandleGetMempoolEntriesByAddresses(context *rpccontext.Context, _ *router.R return errorMessage, nil } - mempoolEntriesByAddresses = append(mempoolEntriesByAddresses, orphanPoolEntriesByAddresse...) + mempoolEntriesByAddresses = append(mempoolEntriesByAddresses, orphanPoolEntriesByAddress...) } return appmessage.NewGetMempoolEntriesByAddressesResponseMessage(mempoolEntriesByAddresses), nil @@ -80,9 +80,13 @@ func extractMempoolEntriesByAddressesFromTransactions(context *rpccontext.Contex for _, transaction := range transactions { for i, input := range transaction.Inputs { - // TODO: Fix this if input.UTXOEntry == nil { - log.Errorf("Couldn't find UTXO entry for input %d in mempool transaction %s. This is a bug and should be fixed.", i, consensushashing.TransactionID(transaction)) + if !areOrphans { // Orphans can legitimately have `input.UTXOEntry == nil` + // TODO: Fix the underlying cause of the bug for non-orphan entries + log.Debugf( + "Couldn't find UTXO entry for input %d in mempool transaction %s. This is a bug and should be fixed.", + i, consensushashing.TransactionID(transaction)) + } continue } diff --git a/app/rpc/rpchandlers/resolve_finality_conflict.go b/app/rpc/rpchandlers/resolve_finality_conflict.go index 0b403149b..1e3f88fb3 100644 --- a/app/rpc/rpchandlers/resolve_finality_conflict.go +++ b/app/rpc/rpchandlers/resolve_finality_conflict.go @@ -8,6 +8,14 @@ import ( // HandleResolveFinalityConflict handles the respectively named RPC command func HandleResolveFinalityConflict(context *rpccontext.Context, _ *router.Router, request appmessage.Message) (appmessage.Message, error) { + if context.Config.SafeRPC { + log.Warn("ResolveFinalityConflict RPC command called while node in safe RPC mode -- ignoring.") + response := &appmessage.ResolveFinalityConflictResponseMessage{} + response.Error = + appmessage.RPCErrorf("ResolveFinalityConflict RPC command called while node in safe RPC mode") + return response, nil + } + response := &appmessage.ResolveFinalityConflictResponseMessage{} response.Error = appmessage.RPCErrorf("not implemented") return response, nil diff --git a/app/rpc/rpchandlers/shut_down.go b/app/rpc/rpchandlers/shut_down.go index c4479f01f..6b7bafdf0 100644 --- a/app/rpc/rpchandlers/shut_down.go +++ b/app/rpc/rpchandlers/shut_down.go @@ -12,6 +12,14 @@ const pauseBeforeShutDown = time.Second // HandleShutDown handles the respectively named RPC command func HandleShutDown(context *rpccontext.Context, _ *router.Router, _ appmessage.Message) (appmessage.Message, error) { + if context.Config.SafeRPC { + log.Warn("ShutDown RPC command called while node in safe RPC mode -- ignoring.") + response := appmessage.NewShutDownResponseMessage() + response.Error = + appmessage.RPCErrorf("ShutDown RPC command called while node in safe RPC mode") + return response, nil + } + log.Warn("ShutDown RPC called.") // Wait a second before shutting down, to allow time to return the response to the caller diff --git a/app/rpc/rpchandlers/unban.go b/app/rpc/rpchandlers/unban.go index 6077daf3e..db3e935d7 100644 --- a/app/rpc/rpchandlers/unban.go +++ b/app/rpc/rpchandlers/unban.go @@ -9,6 +9,14 @@ import ( // HandleUnban handles the respectively named RPC command func HandleUnban(context *rpccontext.Context, _ *router.Router, request appmessage.Message) (appmessage.Message, error) { + if context.Config.SafeRPC { + log.Warn("Unban RPC command called while node in safe RPC mode -- ignoring.") + response := appmessage.NewUnbanResponseMessage() + response.Error = + appmessage.RPCErrorf("Unban RPC command called while node in safe RPC mode") + return response, nil + } + unbanRequest := request.(*appmessage.UnbanRequestMessage) ip := net.ParseIP(unbanRequest.IP) if ip == nil { diff --git a/domain/consensus/model/externalapi/blockinfo.go b/domain/consensus/model/externalapi/blockinfo.go index c7f75a2c3..43c914f92 100644 --- a/domain/consensus/model/externalapi/blockinfo.go +++ b/domain/consensus/model/externalapi/blockinfo.go @@ -13,6 +13,16 @@ type BlockInfo struct { MergeSetReds []*DomainHash } +// HasHeader returns whether the block exists and has a valid header +func (bi *BlockInfo) HasHeader() bool { + return bi.Exists && bi.BlockStatus != StatusInvalid +} + +// HasBody returns whether the block exists and has a valid body +func (bi *BlockInfo) HasBody() bool { + return bi.Exists && bi.BlockStatus != StatusInvalid && bi.BlockStatus != StatusHeaderOnly +} + // Clone returns a clone of BlockInfo func (bi *BlockInfo) Clone() *BlockInfo { return &BlockInfo{ diff --git a/infrastructure/config/config.go b/infrastructure/config/config.go index bba91219b..9b6322e71 100644 --- a/infrastructure/config/config.go +++ b/infrastructure/config/config.go @@ -98,6 +98,7 @@ type Flags struct { RPCMaxWebsockets int `long:"rpcmaxwebsockets" description:"Max number of RPC websocket connections"` RPCMaxConcurrentReqs int `long:"rpcmaxconcurrentreqs" description:"Max number of concurrent RPC requests that may be processed concurrently"` DisableRPC bool `long:"norpc" description:"Disable built-in RPC server"` + SafeRPC bool `long:"saferpc" description:"Disable RPC commands which effect the state of the node"` DisableDNSSeed bool `long:"nodnsseed" description:"Disable DNS seeding for peers"` DNSSeed string `long:"dnsseed" description:"Override DNS seeds with specified hostname (Only 1 hostname allowed)"` GRPCSeed string `long:"grpcseed" description:"Hostname of gRPC server for seeding peers"`