skip CI on some file changes (#712)

* skip running CI on files
* disable http driver for CI workflow, enable it for main workflow
* CI: backport condition change

.github/filters.yaml

@@ -0,0 +1,18 @@
+code:
+  - added|modified: 'go.mod'
+  - added|modified: '.github/filters.yaml'
+  - added|modified: '.github/workflows/ci.yaml'
+  - added|modified: 'build/package/docker/kwild.dockerfile'
+  - added|modified: 'cmd/**'
+  - added|modified: 'common/**'
+  - added|modified: 'core/**/*.go'
+  - added|modified: 'core/!(*.md)'
+  - added|modified: 'core/client/!(*.md)'
+  - added|modified: 'core/client/!(example/**)/**' # this is really weird shit
+  - added|modified: 'core/gatewayclient/!(*.md)'
+  - added|modified: 'core/gatewayclient/!(example/**)/**'
+  - added|modified: 'internal/**'
+  - added|modified: 'parse/**'
+  - added|modified: 'scripts/**/*'
+  - added|modified: 'test/**/*'
+  - 'test/!(stress/**)/**'

.github/workflows/backport.yml

@@ -9,7 +9,7 @@ jobs:
   backport:
     name: Backport PR
     runs-on: ubuntu-latest
-    if: github.event.pull_request.merged && contains(github.event.pull_request.labels.*.name, 'backport-to-')
+    if: contains(github.event.pull_request.labels.*.name, 'backport-to-')
     steps:
       - name: Backport Action
         uses: kwilteam/backport-github-action@b3eae3fb1be75da400e9d7094282dfcd9bc6ffa1 # kwil branch

.github/workflows/ci.yaml

@@ -3,12 +3,44 @@ name: CI
 on:
   pull_request:
     types: [ opened, synchronize, reopened, ready_for_review ]
+    branches:
+      - main
+      - release*
+  # path filter won't work bcz PR status check will be 'pending' if workflow is
+  # skipped due to path filtering, i.e. blocked
+  #paths:
+
   workflow_dispatch:
 
 jobs:
+  # JOB to run change detection
+  changes:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
+    # Set job outputs to values from filter step
+    outputs:
+      run: ${{ steps.filter.outputs.code }}
+    steps:
+      # For pull requests it's not necessary to checkout the code
+      # but it's useful to see the effect of the path filter in current PR
+      - uses: actions/checkout@v4
+        with:
+          # This may save additional git fetch roundtrip if
+          # merge-base is found within latest 20 commits
+          fetch-depth: 20
+      - uses: dorny/[email protected]
+        id: filter
+        with:
+          # Path to file where filters are defined
+          filters: .github/filters.yaml
+          #predicate-quantifier: 'every' # check dorny/paths-filter#225
+
   test:
+    needs: changes
+    #  only run only if there are changes and non-draft PRs
+    if: ${{ needs.changes.outputs.run == 'true' && !github.event.pull_request.draft}}
     runs-on: ubuntu-latest
-    if: ${{ !github.event.pull_request.draft }} # only run on non-draft PRs
 
     services:
       postgres:

.github/workflows/kgw-test-reuse.yaml

@@ -24,7 +24,30 @@ on:
         required: true
 
 jobs:
+  # JOB to run change detection
+  changes:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
+    # Set job outputs to values from filter step
+    outputs:
+      run: ${{ steps.filter.outputs.code }}
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # This may save additional git fetch roundtrip if
+          # merge-base is found within latest 20 commits
+          fetch-depth: 20
+      - uses: dorny/[email protected]
+        id: filter
+        with:
+          filters: .github/filters.yaml
+          #predicate-quantifier: 'every'
+
   kgw-test:
+    needs: changes
+    #  only run only if there are changes, or manually triggered
+    if: ${{ needs.changes.outputs.run == 'true' || github.event_name == 'workflow_dispatch'}}
     runs-on: ubuntu-latest
     steps:
       - name: Show references
@@ -158,6 +181,14 @@ jobs:
         run: |
           docker pull kwilbrennan/extensions-math:multi-arch --platform linux/amd64
 
+      - name: Run integration test on REST API
+        run: |
+          testUserID=$(id -u)
+          testGroupID=$(id -g)
+          cp test/integration/docker-compose.override.yml.example test/integration/docker-compose.override.yml
+          sed -i "s/\${UID}:\${GID}/${testUserID}:${testGroupID}/g" test/integration/docker-compose.override.yml
+          KIT_LOG_LEVEL=warn task test:it:nb -- -drivers http
+
       - name: Run kgw integration test
         run: |
           testUserID=$(id -u)

cmd/kwild/server/build.go

@@ -197,7 +197,10 @@ func buildServer(d *coreDependencies, closers *closeFuncs) *Server {
 	cometBftNode := buildCometNode(d, closers, abciApp)
 
 	cometBftClient := buildCometBftClient(cometBftNode)
-	wrappedCmtClient := &wrappedCometBFTClient{cometBftClient}
+	wrappedCmtClient := &wrappedCometBFTClient{
+		cl:    cometBftClient,
+		cache: abciApp,
+	}
 	txApp.SetReplayStatusChecker(cometBftNode.IsCatchup)
 
 	eventBroadcaster := buildEventBroadcaster(d, ev, wrappedCmtClient, txApp)

cmd/kwild/server/utils.go

@@ -1,11 +1,9 @@
 package server
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
-	"math"
 	"strings"
 
 	types "github.com/kwilteam/kwil-db/core/types/admin"
@@ -55,7 +53,12 @@ func getExtensions(ctx context.Context, urls []string) (map[string]precompiles.I
 // wrappedCometBFTClient satisfies the generic txsvc.BlockchainBroadcaster and
 // admsvc.Node interfaces, hiding the details of cometBFT.
 type wrappedCometBFTClient struct {
-	cl *cmtlocal.Local
+	cl    *cmtlocal.Local
+	cache mempoolCache
+}
+
+type mempoolCache interface {
+	TxInMempool([]byte) bool
 }
 
 func convertNodeInfo(ni *p2p.DefaultNodeInfo) *types.NodeInfo {
@@ -168,23 +171,13 @@ func (wc *wrappedCometBFTClient) TxQuery(ctx context.Context, hash []byte, prove
 		return res, nil
 	}
 
-	// The transaction could be in mempool.
-	limit := math.MaxInt                             // cmt is bugged, -1 doesn't actually work (see rpc/core.validatePerPage and how it goes with 30 instead of no limit)
-	unconf, err := wc.cl.UnconfirmedTxs(ctx, &limit) // SLOW quite often!
-	if err != nil {
-		return nil, err
-	}
-	for _, tx := range unconf.Txs {
-		if bytes.Equal(tx.Hash(), hash) {
-			// Found it. Shoe-horn into a ResultTx with -1 height, and the zero
-			// values for ResponseDeliverTx and TxProof (it's checked and
-			// accepted to mempool, but not delivered in a block yet).
-			return &cmtCoreTypes.ResultTx{
-				Hash:   hash,
-				Height: -1,
-				Tx:     tx,
-			}, nil
-		}
+	// The transaction could be in the mempool, Check with ABCI directly if it heard of the transaction.
+	if wc.cache.TxInMempool(hash) {
+		return &cmtCoreTypes.ResultTx{
+			Hash:   hash,
+			Height: -1,
+			Tx:     nil, // The transaction is still in the mempool, so not indexed yet. Returning nil to avoid hash computations on all the transactions in the mempool (potential DoS attack vector).
+		}, nil
 	}
 	return nil, abci.ErrTxNotFound
 }

core/rpc/client/user/jsonrpc/methods.go

@@ -212,10 +212,16 @@ func (cl *Client) TxQuery(ctx context.Context, txHash []byte) (*transactions.TcT
 	if err != nil {
 		return nil, err
 	}
+
+	var tx transactions.Transaction
+	if res.Tx != nil {
+		tx = *res.Tx
+	}
+
 	return &transactions.TcTxQueryResponse{
 		Hash:     res.Hash,
 		Height:   res.Height,
-		Tx:       *res.Tx,
+		Tx:       tx,
 		TxResult: *res.TxResult,
 	}, nil
 }

internal/abci/abci.go

@@ -48,6 +48,8 @@ func NewAbciApp(cfg *AbciConfig, snapshotter SnapshotModule, statesyncer StateSy
 		log: log,
 
 		validatorAddressToPubKey: make(map[string][]byte),
+
+		txCache: make(map[string]bool),
 	}
 
 	return app
@@ -96,6 +98,13 @@ type AbciApp struct {
 
 	// validatorAddressToPubKey is a map of validator addresses to their public keys
 	validatorAddressToPubKey map[string][]byte
+
+	// txCache is Mempool transactions Cache
+	// This is to store precomputed hashes of all the transactions currently in the mempool
+	// to avoid recomputing the hash for all mempool transactions on every
+	// TxQuery request (to mitigate Potential DDOS attack vector).
+	// https://github.com/kwilteam/kwil-db/issues/714
+	txCache map[string]bool
 }
 
 func (a *AbciApp) ChainID() string {
@@ -150,12 +159,10 @@ func (a *AbciApp) CheckTx(ctx context.Context, incoming *abciTypes.RequestCheckT
 		return &abciTypes.ResponseCheckTx{Code: code.Uint32(), Log: err.Error()}, nil // return error now or is it still all about code?
 	}
 
-	txHash := cmtTypes.Tx(incoming.Tx).Hash()
 	logger.Debug("",
 		zap.String("sender", hex.EncodeToString(tx.Sender)),
 		zap.String("PayloadType", tx.Body.PayloadType.String()),
-		zap.Uint64("nonce", tx.Body.Nonce),
-		zap.String("hash", hex.EncodeToString(txHash)))
+		zap.Uint64("nonce", tx.Body.Nonce))
 
 	// For a new transaction (not re-check), before looking at execution cost or
 	// checking nonce validity, ensure the payload is recognized and signature is valid.
@@ -176,7 +183,7 @@ func (a *AbciApp) CheckTx(ctx context.Context, incoming *abciTypes.RequestCheckT
 			return &abciTypes.ResponseCheckTx{Code: code.Uint32(), Log: err.Error()}, nil
 		}
 	} else {
-		logger.Info("Recheck", zap.String("hash", hex.EncodeToString(txHash)), zap.Uint64("nonce", tx.Body.Nonce), zap.String("payloadType", tx.Body.PayloadType.String()), zap.String("sender", hex.EncodeToString(tx.Sender)))
+		logger.Info("Recheck", zap.String("sender", hex.EncodeToString(tx.Sender)), zap.Uint64("nonce", tx.Body.Nonce), zap.String("payloadType", tx.Body.PayloadType.String()))
 	}
 
 	err = a.txApp.ApplyMempool(ctx, tx)
@@ -197,6 +204,11 @@ func (a *AbciApp) CheckTx(ctx context.Context, incoming *abciTypes.RequestCheckT
 		return &abciTypes.ResponseCheckTx{Code: code.Uint32(), Log: err.Error()}, nil
 	}
 
+	// Cache the transaction hash
+	if newTx {
+		txHash := cmtTypes.Tx(incoming.Tx).Hash()
+		a.txCache[string(txHash)] = true
+	}
 	return &abciTypes.ResponseCheckTx{Code: code.Uint32()}, nil
 }
 
@@ -275,6 +287,10 @@ func (a *AbciApp) FinalizeBlock(ctx context.Context, req *abciTypes.RequestFinal
 		abciRes.GasUsed = txRes.Spend
 
 		res.TxResults = append(res.TxResults, abciRes)
+
+		// Remove the transaction from the cache as it has been included in a block
+		hash := cmtTypes.Tx(tx).Hash()
+		delete(a.txCache, string(hash))
 	}
 
 	res.ConsensusParamUpdates = &tendermintTypes.ConsensusParams{ // why are we "updating" these on every block? Should be nil for no update.
@@ -937,3 +953,8 @@ type EventBroadcaster func(ctx context.Context, proposer []byte) error
 func (a *AbciApp) SetEventBroadcaster(fn EventBroadcaster) {
 	a.broadcastFn = fn
 }
+
+func (a *AbciApp) TxInMempool(txHash []byte) bool {
+	_, ok := a.txCache[string(txHash)]
+	return ok
+}

internal/services/jsonrpc/usersvc/service.go

@@ -440,10 +440,14 @@ func (svc *Service) TxQuery(ctx context.Context, req *jsonrpc.TxQueryRequest) (*
 		return nil, jsonrpc.NewError(jsonrpc.ErrorNodeInternal, "failed to query transaction", nil)
 	}
 
-	tx := &transactions.Transaction{}
-	if err := tx.UnmarshalBinary(cmtResult.Tx); err != nil {
-		logger.Error("failed to deserialize transaction", log.Error(err))
-		return nil, jsonrpc.NewError(jsonrpc.ErrorInternal, "failed to deserialize transaction", nil)
+	//cmtResult.Tx can be nil
+	var tx *transactions.Transaction
+	if cmtResult.Tx != nil {
+		tx = &transactions.Transaction{}
+		if err := tx.UnmarshalBinary(cmtResult.Tx); err != nil {
+			logger.Error("failed to deserialize transaction", log.Error(err))
+			return nil, jsonrpc.NewError(jsonrpc.ErrorInternal, "failed to deserialize transaction", nil)
+		}
 	}
 
 	txResult := &transactions.TransactionResult{

test/acceptance/kwild_test.go

@@ -19,7 +19,7 @@ var remote = flag.Bool("remote", false, "test against remote node")
 
 // NOTE: `-parallel` is a flag that is already used by `go test`
 var parallelMode = flag.Bool("parallel-mode", false, "run tests in parallelMode mode")
-var drivers = flag.String("drivers", "jsonrpc,http,cli", "comma separated list of drivers to run")
+var drivers = flag.String("drivers", "jsonrpc,cli", "comma separated list of drivers to run")
 
 func TestLocalDevSetup(t *testing.T) {
 	if !*dev {

test/integration/kwild_test.go

@@ -18,7 +18,7 @@ var dev = flag.Bool("dev", false, "run for development purpose (no tests)")
 
 var spamTest = flag.Bool("spam", false, "run the spam test that requires a special docker image to be built")
 
-var drivers = flag.String("drivers", "jsonrpc,http,cli", "comma separated list of drivers to run")
+var drivers = flag.String("drivers", "jsonrpc,cli", "comma separated list of drivers to run")
 
 // NOTE: `-parallel` is a flag that is already used by `go test`
 var parallelMode = flag.Bool("parallel-mode", false, "run tests in parallel mode")