From 35e8b383a3cb4d9c7a24a9d1e87ee79322c65c7d Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 16 Jul 2024 12:31:52 -0400 Subject: [PATCH 1/4] feat(faucet): ensure successful transfer before responding - closes: #417 --- starship/faucet/handler.go | 67 +++++++++++++++++++++++++-- starship/tests/e2e/faucet_test.go | 77 ++++++++++++++++++++++++++----- 2 files changed, 127 insertions(+), 17 deletions(-) diff --git a/starship/faucet/handler.go b/starship/faucet/handler.go index 08bf0d678..c41bd75f8 100644 --- a/starship/faucet/handler.go +++ b/starship/faucet/handler.go @@ -2,7 +2,11 @@ package main import ( "context" + "fmt" + "math/big" + "time" + "go.uber.org/zap" "google.golang.org/protobuf/types/known/emptypb" pb "github.com/cosmology-tech/starship/faucet/faucet" @@ -37,11 +41,64 @@ func (a *AppServer) Status(ctx context.Context, _ *emptypb.Empty) (*pb.State, er return state, nil } +func (a *AppServer) getBalance(address, denom string) (*big.Int, error) { + account := &Account{config: a.config, logger: a.logger, Address: address} + coin, err := account.GetBalanceByDenom(denom) + if err != nil { + // Log the error, but don't return it + a.logger.Debug("Error getting balance, assuming new account", zap.Error(err)) + return new(big.Int), nil // Return 0 balance + } + balance, ok := new(big.Int).SetString(coin.Amount, 10) + if !ok { + return nil, fmt.Errorf("failed to parse balance") + } + return balance, nil +} + func (a *AppServer) Credit(ctx context.Context, requestCredit *pb.RequestCredit) (*pb.ResponseCredit, error) { - err := a.distributor.SendTokens(requestCredit.GetAddress(), requestCredit.GetDenom()) - if err != nil { - return nil, err - } + // Get initial balance before sending tokens + initialBalance, err := a.getBalance(requestCredit.GetAddress(), requestCredit.GetDenom()) + if err != nil { + return nil, fmt.Errorf("failed to get initial balance: %v", err) + } + + err = a.distributor.SendTokens(requestCredit.GetAddress(), requestCredit.GetDenom()) + if err != nil { + return nil, err + } + + // Check balance after transfer + confirmed, err := a.confirmBalanceUpdate(requestCredit.GetAddress(), requestCredit.GetDenom(), initialBalance) + if err != nil { + return &pb.ResponseCredit{Status: fmt.Sprintf("error: %v", err)}, err + } + if !confirmed { + return &pb.ResponseCredit{Status: "error: failed to confirm balance update (timeout)"}, nil + } + + return &pb.ResponseCredit{Status: "ok"}, nil +} + +func (a *AppServer) confirmBalanceUpdate(address, denom string, initialBalance *big.Int) (bool, error) { + expectedIncrease, ok := new(big.Int).SetString(a.distributor.CreditCoins.GetDenomAmount(denom), 10) + if !ok { + return false, fmt.Errorf("failed to parse expected amount") + } + + expectedFinalBalance := new(big.Int).Add(initialBalance, expectedIncrease) - return &pb.ResponseCredit{Status: "ok"}, nil + for i := 0; i < 3; i++ { // Try 3 times with 5-second intervals + currentBalance, err := a.getBalance(address, denom) + if err != nil { + return false, err + } + if currentBalance.Cmp(expectedFinalBalance) >= 0 { + return true, nil + } + if i < 2 { + time.Sleep(5 * time.Second) + } + } + return false, nil } diff --git a/starship/tests/e2e/faucet_test.go b/starship/tests/e2e/faucet_test.go index cbe0738a7..29923de12 100644 --- a/starship/tests/e2e/faucet_test.go +++ b/starship/tests/e2e/faucet_test.go @@ -4,11 +4,12 @@ import ( "bytes" "encoding/json" "fmt" - pb "github.com/cosmology-tech/starship/registry/registry" "net/http" urlpkg "net/url" "strconv" "time" + + pb "github.com/cosmology-tech/starship/registry/registry" ) func (s *TestSuite) MakeFaucetRequest(chain *Chain, req *http.Request, unmarshal map[string]interface{}) { @@ -115,6 +116,28 @@ func (s *TestSuite) getAccountBalance(chain *Chain, address string, denom string return float64(0) } +func (s *TestSuite) creditAccount(chain *Chain, addr, denom string) error { + body := map[string]string{ + "denom": denom, + "address": addr, + } + postBody, err := json.Marshal(body) + if err != nil { + return err + } + resp, err := http.Post( + fmt.Sprintf("http://0.0.0.0:%d/credit", chain.Ports.Faucet), + "application/json", + bytes.NewBuffer(postBody)) + if err != nil { + return err + } + if resp.StatusCode != 200 { + return fmt.Errorf("unexpected status code: %d", resp.StatusCode) + } + return nil +} + func (s *TestSuite) TestFaucet_Credit() { s.T().Log("running test for /credit endpoint for faucet") @@ -132,18 +155,8 @@ func (s *TestSuite) TestFaucet_Credit() { addr := getAddressFromType(chain.Name) beforeBalance := s.getAccountBalance(chain, addr, denom) - body := map[string]string{ - "denom": denom, - "address": addr, - } - postBody, err := json.Marshal(body) - s.Require().NoError(err) - resp, err := http.Post( - fmt.Sprintf("http://0.0.0.0:%d/credit", chain.Ports.Faucet), - "application/json", - bytes.NewBuffer(postBody)) + err := s.creditAccount(chain, addr, denom) s.Require().NoError(err) - s.Require().Equal(200, resp.StatusCode) time.Sleep(4 * time.Second) afterBalance := s.getAccountBalance(chain, addr, denom) @@ -154,3 +167,43 @@ func (s *TestSuite) TestFaucet_Credit() { }) } } + +func (s *TestSuite) TestFaucet_Credit_MultipleRequests() { + s.T().Log("running test for multiple requests to /credit endpoint for faucet") + + // expected amount to be credited via faucet + expCreditedAmt := float64(10000000000) + + for _, chain := range s.config.Chains { + s.Run(fmt.Sprintf("multiple faucet requests test for: %s", chain.ID), func() { + if chain.Ports.Faucet == 0 { + s.T().Skip("faucet not exposed via ports") + } + + // fetch denom and address from an account on chain + denom := s.getChainDenoms(chain) + addr := getAddressFromType(chain.Name) + beforeBalance := s.getAccountBalance(chain, addr, denom) + + // Send multiple requests + numRequests := 3 + for i := 0; i < numRequests; i++ { + err := s.creditAccount(chain, addr, denom) + s.Require().NoError(err) + } + + // Allow more time for processing multiple requests + time.Sleep(15 * time.Second) + + afterBalance := s.getAccountBalance(chain, addr, denom) + s.T().Log("address:", addr, "after balance: ", afterBalance, "before balance:", beforeBalance) + + // Check that the balance has increased by at least the expected amount times the number of requests + expectedIncrease := expCreditedAmt * float64(numRequests) + actualIncrease := afterBalance - beforeBalance + s.Require().GreaterOrEqual(actualIncrease, expectedIncrease, + "Balance didn't increase as expected. Actual increase: %f, Expected increase: %f", + actualIncrease, expectedIncrease) + }) + } +} From 4934e3f78ebe212804b99f2b89247d68d567cc60 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 16 Jul 2024 13:13:27 -0400 Subject: [PATCH 2/4] fixup! feat(faucet): ensure successful transfer before responding --- starship/tests/e2e/faucet_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/starship/tests/e2e/faucet_test.go b/starship/tests/e2e/faucet_test.go index 29923de12..4a699f616 100644 --- a/starship/tests/e2e/faucet_test.go +++ b/starship/tests/e2e/faucet_test.go @@ -7,7 +7,6 @@ import ( "net/http" urlpkg "net/url" "strconv" - "time" pb "github.com/cosmology-tech/starship/registry/registry" ) @@ -158,7 +157,6 @@ func (s *TestSuite) TestFaucet_Credit() { err := s.creditAccount(chain, addr, denom) s.Require().NoError(err) - time.Sleep(4 * time.Second) afterBalance := s.getAccountBalance(chain, addr, denom) s.T().Log("address:", addr, "after balance: ", afterBalance, "before balance:", beforeBalance) // note sometimes expected difference is 9x expected value (bug due to using holder address for test) @@ -192,9 +190,6 @@ func (s *TestSuite) TestFaucet_Credit_MultipleRequests() { s.Require().NoError(err) } - // Allow more time for processing multiple requests - time.Sleep(15 * time.Second) - afterBalance := s.getAccountBalance(chain, addr, denom) s.T().Log("address:", addr, "after balance: ", afterBalance, "before balance:", beforeBalance) From 2cf5110b3278f303a9ba359a20bc6c7979e6a9b3 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Tue, 13 Aug 2024 20:06:08 +0530 Subject: [PATCH 3/4] add function to fetch and parse the txhash from send token --- starship/faucet/distributor.go | 43 +++++++++++++++++++++++++++++++++- starship/faucet/utils.go | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/starship/faucet/distributor.go b/starship/faucet/distributor.go index b06ed0ffa..f4027c271 100644 --- a/starship/faucet/distributor.go +++ b/starship/faucet/distributor.go @@ -271,6 +271,23 @@ func (a *Account) String() string { return fmt.Sprintf("name: %s, addr: %s", a.Name, a.Address) } +// queryTx queries the tx based on the txhash +func (a *Account) queryTx(txhash string) (map[string]interface{}, error) { + cmdStr := fmt.Sprintf("%s q tx %s --output=json", a.config.ChainBinary, txhash) + output, err := runCommand(cmdStr) + if err != nil { + return nil, err + } + + txMap := map[string]interface{}{} + err = json.Unmarshal(output, &txMap) + if err != nil { + return nil, err + } + + return txMap, nil +} + // sendTokens performs chain binary send txn from account func (a *Account) sendTokens(address string, denom string, amount string) error { ok := a.mu.TryLock() @@ -279,7 +296,7 @@ func (a *Account) sendTokens(address string, denom string, amount string) error } defer a.mu.Unlock() - args := fmt.Sprintf("--chain-id=%s --fees=%s --keyring-backend=test --gas=auto --gas-adjustment=1.5 --yes --node=%s", a.config.ChainId, a.config.ChainFees, a.config.ChainRPCEndpoint) + args := fmt.Sprintf("--chain-id=%s --fees=%s --keyring-backend=test --gas=auto --gas-adjustment=1.5 --output=json --yes --node=%s", a.config.ChainId, a.config.ChainFees, a.config.ChainRPCEndpoint) cmdStr := fmt.Sprintf("%s tx bank send %s %s %s%s %s", a.config.ChainBinary, a.Address, address, amount, denom, args) output, err := runCommand(cmdStr) if err != nil { @@ -288,6 +305,30 @@ func (a *Account) sendTokens(address string, denom string, amount string) error } a.logger.Info("ran cmd to send tokens", zap.String("cmd", cmdStr), zap.String("stdout", string(output))) + // Find the JSON line and extract the txhash using a regular expression + txhash, err := extractTxHash(string(output)) + if err != nil { + a.logger.Error("failed to extract txhash", zap.Error(err)) + return err + } + + a.logger.Debug("send tokens txhash", zap.String("txhash", txhash)) + + // query tx to check if the tx was successful + txMap, err := a.queryTx(txhash) + if err != nil { + return err + } + + // check if the tx has the event + err = hasEvent(txMap, "transfer") + if err != nil { + a.logger.Error("transfer event not found in tx", + zap.String("txhash", txhash), + zap.Any("txMap", txMap)) + return err + } + return nil } diff --git a/starship/faucet/utils.go b/starship/faucet/utils.go index 928d5399a..fd1798d5c 100644 --- a/starship/faucet/utils.go +++ b/starship/faucet/utils.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/exec" + "regexp" ) // runCommand will run the cmdStr as exec and return the results as a []byte @@ -22,3 +23,45 @@ func runCommand(cmdStr string) ([]byte, error) { return outb.Bytes(), nil } + +// extractTxHash uses a regular expression to extract the txhash from the output +func extractTxHash(output string) (string, error) { + // Regular expression to match the txhash value in the JSON output + re := regexp.MustCompile(`"txhash":"([0-9A-Fa-f]{64})"`) + match := re.FindStringSubmatch(output) + if len(match) < 2 { + return "", fmt.Errorf("txhash not found in output") + } + return match[1], nil +} + +// hasEvent from txResults txMap. Returns nil if the event is found, otherwise an error +func hasEvent(txResults map[string]interface{}, eventType string) error { + // Check for the "transfer" event + logs, ok := txResults["logs"].([]interface{}) + if !ok || len(logs) == 0 { + return fmt.Errorf("no logs found in transaction") + } + + for _, log := range logs { + logMap, ok := log.(map[string]interface{}) + if !ok { + continue + } + events, ok := logMap["events"].([]interface{}) + if !ok { + continue + } + for _, event := range events { + eventMap, ok := event.(map[string]interface{}) + if !ok { + continue + } + if eventMap["type"] == eventType { + return nil + } + } + } + + return fmt.Errorf("event %s not found in transaction", eventType) +} From d729757f082e1775eebbc13ee5981dc1b45a1237 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Tue, 13 Aug 2024 20:30:33 +0530 Subject: [PATCH 4/4] update hasEvent function --- starship/faucet/utils.go | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/starship/faucet/utils.go b/starship/faucet/utils.go index fd1798d5c..6edb78d60 100644 --- a/starship/faucet/utils.go +++ b/starship/faucet/utils.go @@ -35,23 +35,29 @@ func extractTxHash(output string) (string, error) { return match[1], nil } -// hasEvent from txResults txMap. Returns nil if the event is found, otherwise an error +// hasEvent checks for a specific event type in the transaction results. +// It first tries to fetch the events either from txMap["events"] or from txMap["logs"] and then checks for the event type. func hasEvent(txResults map[string]interface{}, eventType string) error { - // Check for the "transfer" event - logs, ok := txResults["logs"].([]interface{}) - if !ok || len(logs) == 0 { - return fmt.Errorf("no logs found in transaction") - } + var events []interface{} - for _, log := range logs { - logMap, ok := log.(map[string]interface{}) - if !ok { - continue - } - events, ok := logMap["events"].([]interface{}) - if !ok { - continue + // Attempt to fetch events directly from txMap["events"] + if directEvents, ok := txResults["events"].([]interface{}); ok && len(directEvents) > 0 { + events = directEvents + } else if logs, ok := txResults["logs"].([]interface{}); ok && len(logs) > 0 { + // If no direct events, attempt to fetch events from logs + for _, log := range logs { + logMap, ok := log.(map[string]interface{}) + if !ok { + continue + } + if logEvents, ok := logMap["events"].([]interface{}); ok && len(logEvents) > 0 { + events = append(events, logEvents...) + } } + } + + // If events are found, check for the specific eventType + if len(events) > 0 { for _, event := range events { eventMap, ok := event.(map[string]interface{}) if !ok { @@ -61,7 +67,9 @@ func hasEvent(txResults map[string]interface{}, eventType string) error { return nil } } + return fmt.Errorf("event %s not found in transaction events", eventType) } - return fmt.Errorf("event %s not found in transaction", eventType) + // If no events were found in either place + return fmt.Errorf("no events found in transaction") }