From 8d1e8364a3be5835f53e407cbbe1ae80b321f821 Mon Sep 17 00:00:00 2001 From: Daulet Dulanov Date: Tue, 19 Jul 2022 02:55:25 +0200 Subject: [PATCH 1/6] [api] impl. TestGrpcServer_GetServerMeta (#3559) --- api/grpcserver_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/api/grpcserver_test.go b/api/grpcserver_test.go index 91b0c280f9..684304a456 100644 --- a/api/grpcserver_test.go +++ b/api/grpcserver_test.go @@ -104,7 +104,20 @@ func TestGrpcServer_GetReceiptByAction(t *testing.T) { } func TestGrpcServer_GetServerMeta(t *testing.T) { + require := require.New(t) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + core := mock_apicoreservice.NewMockCoreService(ctrl) + grpcSvr := newGRPCHandler(core) + core.EXPECT().ServerMeta().Return("packageVersion", "packageCommitID", "gitStatus", "goVersion", "buildTime") + res, err := grpcSvr.GetServerMeta(context.Background(), &iotexapi.GetServerMetaRequest{}) + require.NoError(err) + require.Equal("packageVersion", res.ServerMeta.PackageVersion) + require.Equal("packageCommitID", res.ServerMeta.PackageCommitID) + require.Equal("gitStatus", res.ServerMeta.GitStatus) + require.Equal("goVersion", res.ServerMeta.GoVersion) + require.Equal("buildTime", res.ServerMeta.BuildTime) } func TestGrpcServer_ReadContract(t *testing.T) { From 378185ab78a6e64117d5ad466c4e71808f7b63f3 Mon Sep 17 00:00:00 2001 From: huofei <68298506@qq.com> Date: Tue, 19 Jul 2022 12:52:09 +0800 Subject: [PATCH 2/6] fix uncontrolled data used in path expression (#3547) --- ioctl/cmd/contract/contractshare.go | 61 ++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/ioctl/cmd/contract/contractshare.go b/ioctl/cmd/contract/contractshare.go index 704251410c..098c93a437 100644 --- a/ioctl/cmd/contract/contractshare.go +++ b/ioctl/cmd/contract/contractshare.go @@ -117,7 +117,7 @@ func isExist(path string) bool { } func rename(oldPath string, newPath string, c chan bool) { - if isExist(_givenPath + "/" + oldPath) { + if isExist(oldPath) { if err := os.Rename(oldPath, newPath); err != nil { log.Println("Rename file failed: ", err) } @@ -127,7 +127,7 @@ func rename(oldPath string, newPath string, c chan bool) { } func share(args []string) error { - _givenPath = args[0] + _givenPath = filepath.Clean(args[0]) if len(_givenPath) == 0 { return output.NewError(output.ReadFileError, "failed to get directory", nil) } @@ -186,7 +186,7 @@ func share(args []string) error { case "list": payload := make(map[string]bool) for _, ele := range _fileList { - payload[ele] = isReadOnly(_givenPath + "/" + ele) + payload[ele] = isReadOnly(filepath.Join(_givenPath, ele)) } response.Payload = payload if err := conn.WriteJSON(&response); err != nil { @@ -197,48 +197,73 @@ func share(args []string) error { t := request.Payload getPayload := reflect.ValueOf(t).Index(0).Interface().(map[string]interface{}) - getPayloadPath := getPayload["path"].(string) - upload, err := os.ReadFile(filepath.Clean(_givenPath + "/" + getPayloadPath)) + getPayloadPath, err := cleanPath(getPayload["path"].(string)) + if err != nil { + log.Println("clean file path failed: ", err) + break + } + getPayloadPath = filepath.Clean(filepath.Join(_givenPath, getPayloadPath)) + upload, err := os.ReadFile(getPayloadPath) if err != nil { log.Println("read file failed: ", err) + break } payload["content"] = string(upload) - payload["readonly"] = isReadOnly(_givenPath + "/" + getPayloadPath) + payload["readonly"] = isReadOnly(getPayloadPath) response.Payload = payload if err := conn.WriteJSON(&response); err != nil { log.Println("send get response: ", err) break } - log.Println("share: " + _givenPath + "/" + getPayloadPath) + log.Println("share: " + getPayloadPath) case "rename": c := make(chan bool) t := request.Payload renamePayload := reflect.ValueOf(t).Index(0).Interface().(map[string]interface{}) - oldPath := renamePayload["oldPath"].(string) - newPath := renamePayload["newPath"].(string) - go rename(oldPath, newPath, c) + oldRenamePath, err := cleanPath(renamePayload["oldPath"].(string)) + if err != nil { + log.Println("clean file path failed: ", err) + break + } + newRenamePath, err := cleanPath(renamePayload["newPath"].(string)) + if err != nil { + log.Println("clean file path failed: ", err) + break + } + oldRenamePath = filepath.Join(_givenPath, oldRenamePath) + newRenamePath = filepath.Join(_givenPath, newRenamePath) + go rename(oldRenamePath, newRenamePath, c) response.Payload = <-c if err := conn.WriteJSON(&response); err != nil { log.Println("send get response: ", err) break } - log.Println("rename: " + _givenPath + "/" + oldPath + " to " + _givenPath + "/" + newPath) + log.Println("rename: " + oldRenamePath + " to " + newRenamePath) case "set": t := request.Payload setPayload := reflect.ValueOf(t).Index(0).Interface().(map[string]interface{}) - setPath := setPayload["path"].(string) content := setPayload["content"].(string) - err := os.WriteFile(_givenPath+"/"+setPath, []byte(content), 0777) + setPath, err := cleanPath(setPayload["path"].(string)) if err != nil { + log.Println("clean file path failed: ", err) + break + } + setPath = filepath.Join(_givenPath, setPath) + if err := os.MkdirAll(filepath.Dir(setPath), 0755); err != nil { + log.Println("mkdir failed: ", err) + break + } + if err := os.WriteFile(setPath, []byte(content), 0644); err != nil { log.Println("set file failed: ", err) + break } if err := conn.WriteJSON(&response); err != nil { log.Println("send set response: ", err) break } - log.Println("set: " + _givenPath + "/" + setPath) + log.Println("set: " + setPath) default: log.Println("Don't support this IDE yet. Can not handle websocket method: " + request.Key) @@ -249,5 +274,13 @@ func share(args []string) error { log.Fatal(http.ListenAndServe(*_addr, nil)) return nil +} +func cleanPath(path string) (string, error) { + path = filepath.Clean(filepath.Join("/", path)) + real, err := filepath.Rel("/", path) + if err != nil { + return "", err + } + return real, nil } From 5a9ab76985eed6722b9d0a9b8ece344d54b04af9 Mon Sep 17 00:00:00 2001 From: Haaai <55118568+Liuhaai@users.noreply.github.com> Date: Mon, 18 Jul 2022 22:22:53 -0700 Subject: [PATCH 3/6] add log in rolldposctx (#3553) Co-authored-by: huofei <68298506@qq.com> --- consensus/consensus.go | 2 +- consensus/scheme/rolldpos/rolldposctx.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/consensus/consensus.go b/consensus/consensus.go index 860b3ce6e1..d19f369431 100644 --- a/consensus/consensus.go +++ b/consensus/consensus.go @@ -161,7 +161,7 @@ func NewConsensus( commitBlockCB := func(blk *block.Block) error { err := bc.CommitBlock(blk) if err != nil { - log.Logger("consensus").Info("Failed to commit the block.", zap.Error(err), zap.Uint64("height", blk.Height())) + log.Logger("consensus").Error("Failed to commit the block.", zap.Error(err), zap.Uint64("height", blk.Height())) } return err } diff --git a/consensus/scheme/rolldpos/rolldposctx.go b/consensus/scheme/rolldpos/rolldposctx.go index a86f9f1547..06a3c875ab 100644 --- a/consensus/scheme/rolldpos/rolldposctx.go +++ b/consensus/scheme/rolldpos/rolldposctx.go @@ -486,6 +486,7 @@ func (ctx *rollDPoSCtx) Commit(msg interface{}) (bool, error) { case nil: break default: + log.L().Error("error when committing the block", zap.Error(err)) return false, errors.Wrap(err, "error when committing a block") } // Broadcast the committed block to the network From db12104b264e3632bd8b8289725061866eeda5cb Mon Sep 17 00:00:00 2001 From: huofei <68298506@qq.com> Date: Tue, 19 Jul 2022 14:08:17 +0800 Subject: [PATCH 4/6] [ioctl] fix log entries created from user input (#3546) * fix log entries created from user input * resolve conflict --- ioctl/cmd/contract/contractshare.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ioctl/cmd/contract/contractshare.go b/ioctl/cmd/contract/contractshare.go index 098c93a437..0827cf5d0a 100644 --- a/ioctl/cmd/contract/contractshare.go +++ b/ioctl/cmd/contract/contractshare.go @@ -154,7 +154,7 @@ func share(args []string) error { return nil }) - log.Println("Listening on 127.0.0.1:65520, Please open your IDE ( " + _iotexIDE + " ) to connect to local files") + log.Printf("Listening on 127.0.0.1:65520, Please open your IDE ( %s ) to connect to local files", _iotexIDE) http.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { conn, err := _upgrade.Upgrade(writer, request, nil) @@ -202,7 +202,7 @@ func share(args []string) error { log.Println("clean file path failed: ", err) break } - getPayloadPath = filepath.Clean(filepath.Join(_givenPath, getPayloadPath)) + getPayloadPath = filepath.Join(_givenPath, getPayloadPath) upload, err := os.ReadFile(getPayloadPath) if err != nil { log.Println("read file failed: ", err) @@ -215,7 +215,7 @@ func share(args []string) error { log.Println("send get response: ", err) break } - log.Println("share: " + getPayloadPath) + log.Printf("share: %s\n", easpcapeString(getPayloadPath)) case "rename": c := make(chan bool) @@ -239,7 +239,7 @@ func share(args []string) error { log.Println("send get response: ", err) break } - log.Println("rename: " + oldRenamePath + " to " + newRenamePath) + log.Printf("rename: %s to %s\n", easpcapeString(oldRenamePath), easpcapeString(newRenamePath)) case "set": t := request.Payload @@ -263,10 +263,10 @@ func share(args []string) error { log.Println("send set response: ", err) break } - log.Println("set: " + setPath) + log.Printf("set: %s\n", easpcapeString(setPath)) default: - log.Println("Don't support this IDE yet. Can not handle websocket method: " + request.Key) + log.Printf("Don't support this IDE yet. Can not handle websocket method: %s\n", easpcapeString(request.Key)) } } @@ -276,6 +276,11 @@ func share(args []string) error { return nil } +func easpcapeString(str string) string { + escaped := strings.Replace(str, "\n", "", -1) + return strings.Replace(escaped, "\r", "", -1) +} + func cleanPath(path string) (string, error) { path = filepath.Clean(filepath.Join("/", path)) real, err := filepath.Rel("/", path) From 2ed5b0387338860ed93331c19f9dc589f21da94e Mon Sep 17 00:00:00 2001 From: millken Date: Tue, 19 Jul 2022 15:29:44 +0800 Subject: [PATCH 5/6] move chanid metrics to chainservice (#3544) Co-authored-by: dustinxie --- api/grpcserver.go | 38 --------------------------- api/grpcserver_test.go | 1 - chainservice/chainservice.go | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/api/grpcserver.go b/api/grpcserver.go index 1a7ca92703..ff242ec30f 100644 --- a/api/grpcserver.go +++ b/api/grpcserver.go @@ -26,7 +26,6 @@ import ( "github.com/iotexproject/iotex-proto/golang/iotexapi" "github.com/iotexproject/iotex-proto/golang/iotextypes" "github.com/pkg/errors" - "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel/attribute" "go.uber.org/zap" @@ -35,7 +34,6 @@ import ( "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/keepalive" - "google.golang.org/grpc/peer" "google.golang.org/grpc/reflection" "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" @@ -72,28 +70,8 @@ var ( Time: 60 * time.Second, // Ping the client if it is idle for 60 seconds to ensure the connection is still active Timeout: 10 * time.Second, // Wait 10 seconds for the ping ack before assuming the connection is dead } - - _apiCallSourceWithChainIDMtc = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "iotex_apicallsource_chainid_metrics", - Help: "API call Source ChainID Statistics", - }, - []string{"chain_id"}, - ) - _apiCallSourceWithOutChainIDMtc = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "iotex_apicallsource_nochainid_metrics", - Help: "API call Source Without ChainID Statistics", - }, - []string{"client_ip", "sender"}, - ) ) -func init() { - prometheus.MustRegister(_apiCallSourceWithChainIDMtc) - prometheus.MustRegister(_apiCallSourceWithOutChainIDMtc) -} - // RecoveryInterceptor handles panic to a custom error func RecoveryInterceptor() grpc_recovery.Option { return grpc_recovery.WithRecoveryHandler(func(p interface{}) (err error) { @@ -342,22 +320,6 @@ func (svr *gRPCHandler) SendAction(ctx context.Context, in *iotexapi.SendActionR // tags output span.SetAttributes(attribute.String("actType", fmt.Sprintf("%T", in.GetAction().GetCore()))) defer span.End() - chainID := strconv.FormatUint(uint64(in.GetAction().GetCore().GetChainID()), 10) - if in.GetAction().GetCore().GetChainID() > 0 { - _apiCallSourceWithChainIDMtc.WithLabelValues(chainID).Inc() - } else { - selp, err := (&action.Deserializer{}).SetEvmNetworkID(svr.coreService.EVMNetworkID()).ActionToSealedEnvelope(in.GetAction()) - if err != nil { - return nil, err - } - var clientIP string - if p, ok := peer.FromContext(ctx); ok { - clientIP, _, _ = net.SplitHostPort(p.Addr.String()) - } else { - clientIP = "unknownIP" - } - _apiCallSourceWithOutChainIDMtc.WithLabelValues(clientIP, selp.SenderAddress().String()).Inc() - } actHash, err := svr.coreService.SendAction(ctx, in.GetAction()) if err != nil { return nil, err diff --git a/api/grpcserver_test.go b/api/grpcserver_test.go index 684304a456..5145aab049 100644 --- a/api/grpcserver_test.go +++ b/api/grpcserver_test.go @@ -86,7 +86,6 @@ func TestGrpcServer_SendAction(t *testing.T) { grpcSvr := newGRPCHandler(core) for _, test := range _sendActionTests { - core.EXPECT().EVMNetworkID().Return(uint32(1)) core.EXPECT().SendAction(context.Background(), test.actionPb).Return(test.actionHash, nil) request := &iotexapi.SendActionRequest{Action: test.actionPb} res, err := grpcSvr.SendAction(context.Background(), request) diff --git a/chainservice/chainservice.go b/chainservice/chainservice.go index 823c226156..515429d73f 100644 --- a/chainservice/chainservice.go +++ b/chainservice/chainservice.go @@ -8,11 +8,14 @@ package chainservice import ( "context" + "strconv" "github.com/libp2p/go-libp2p-core/peer" "github.com/pkg/errors" + "github.com/prometheus/client_golang/prometheus" "google.golang.org/protobuf/proto" + "github.com/iotexproject/iotex-address/address" "github.com/iotexproject/iotex-election/committee" "github.com/iotexproject/iotex-proto/golang/iotexrpc" "github.com/iotexproject/iotex-proto/golang/iotextypes" @@ -36,6 +39,28 @@ import ( "github.com/iotexproject/iotex-core/state/factory" ) +var ( + _apiCallWithChainIDMtc = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "iotex_apicall_chainid_metrics", + Help: "API call ChainID Statistics", + }, + []string{"chain_id"}, + ) + _apiCallWithOutChainIDMtc = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "iotex_apicall_nochainid_metrics", + Help: "API call Without ChainID Statistics", + }, + []string{"sender", "recipient"}, + ) +) + +func init() { + prometheus.MustRegister(_apiCallWithChainIDMtc) + prometheus.MustRegister(_apiCallWithOutChainIDMtc) +} + // ChainService is a blockchain service with all blockchain components. type ChainService struct { lifecycle lifecycle.Lifecycle @@ -80,9 +105,34 @@ func (cs *ChainService) HandleAction(ctx context.Context, actPb *iotextypes.Acti if err != nil { log.L().Debug(err.Error()) } + chainIDmetrics(act) return err } +func chainIDmetrics(act action.SealedEnvelope) { + chainID := strconv.FormatUint(uint64(act.ChainID()), 10) + if act.ChainID() > 0 { + _apiCallWithChainIDMtc.WithLabelValues(chainID).Inc() + } else { + recipient, _ := act.Destination() + //it will be empty for staking action, change string to staking in such case + if recipient == "" { + act, ok := act.Action().(action.EthCompatibleAction) + if ok { + if ethTx, err := act.ToEthTx(); err == nil && ethTx.To() != nil { + if add, err := address.FromHex(ethTx.To().Hex()); err == nil { + recipient = add.String() + } + } + } + if recipient == "" { + recipient = "staking" + } + } + _apiCallWithOutChainIDMtc.WithLabelValues(act.SenderAddress().String(), recipient).Inc() + } +} + // HandleBlock handles incoming block request. func (cs *ChainService) HandleBlock(ctx context.Context, peer string, pbBlock *iotextypes.Block) error { blk, err := block.NewDeserializer(cs.chain.EvmNetworkID()).FromBlockProto(pbBlock) From 05c7def60cfc4fc22e712afc4268b85ecab8b80b Mon Sep 17 00:00:00 2001 From: huof6890 <68298506@qq.com> Date: Tue, 19 Jul 2022 16:47:10 +0800 Subject: [PATCH 6/6] modify test --- ioctl/newcmd/config/config_test.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/ioctl/newcmd/config/config_test.go b/ioctl/newcmd/config/config_test.go index 87ae60cf9e..4a1445e74a 100644 --- a/ioctl/newcmd/config/config_test.go +++ b/ioctl/newcmd/config/config_test.go @@ -31,15 +31,15 @@ func TestInitConfig(t *testing.T) { func TestConfigGet(t *testing.T) { require := require.New(t) testPath := t.TempDir() - _configDir = testPath - cfg, cfgFilePath, err := InitConfig() - require.NoError(err) - // the endpoint & default account are blank strings within the initial config, so I'm setting it here - cfg.Endpoint = "http://google.com" - cfg.DefaultAccount = config.Context{AddressOrAlias: "test"} - require.NoError(err) - - info := newInfo(cfg, cfgFilePath) + info := newInfo(config.Config{ + Wallet: testPath, + SecureConnect: true, + Aliases: make(map[string]string), + DefaultAccount: config.Context{AddressOrAlias: "test"}, + Explorer: "iotexscan", + Language: "English", + AnalyserEndpoint: "testAnalyser", + }, testPath) tcs := []struct { arg string @@ -47,7 +47,7 @@ func TestConfigGet(t *testing.T) { }{ { "endpoint", - "http://google.com secure connect(TLS):true", + "no endpoint has been set", }, { "wallet", @@ -67,15 +67,15 @@ func TestConfigGet(t *testing.T) { }, { "nsv2height", - "5165641", + "0", }, { "analyserEndpoint", - "https://iotex-analyser-api-mainnet.chainanalytics.org", + "testAnalyser", }, { "all", - "\"endpoint\": \"http://google.com\",\n \"secureConnect\": true,\n \"aliases\": {},\n \"defaultAccount\": {\n \"addressOrAlias\": \"test\"\n },\n \"explorer\": \"iotexscan\",\n \"language\": \"English\",\n \"nsv2height\": 5165641,\n \"analyserEndpoint\": \"https://iotex-analyser-api-mainnet.chainanalytics.org\"\n}", + "\"endpoint\": \"\",\n \"secureConnect\": true,\n \"aliases\": {},\n \"defaultAccount\": {\n \"addressOrAlias\": \"test\"\n },\n \"explorer\": \"iotexscan\",\n \"language\": \"English\",\n \"nsv2height\": 0,\n \"analyserEndpoint\": \"testAnalyser\"\n}", }, } @@ -83,8 +83,9 @@ func TestConfigGet(t *testing.T) { cfgItem, err := info.get(tc.arg) if err != nil { require.Contains(err.Error(), tc.expected) + } else { + require.Contains(cfgItem, tc.expected) } - require.Contains(cfgItem, tc.expected) } }