Rebase fork onto v0.47.3

.github/workflows/sims.yml

@@ -88,7 +88,7 @@ jobs:
     needs:
       [test-sim-multi-seed-short, test-sim-after-import, test-sim-import-export]
     runs-on: ubuntu-latest
-    if: ${{ success() }}
+    if: ${{ false }} # Disabled due to requiring Slack integration
     steps:
       - name: Check out repository
         uses: actions/checkout@v3
@@ -116,7 +116,7 @@ jobs:
     needs:
       [test-sim-multi-seed-short, test-sim-after-import, test-sim-import-export]
     runs-on: ubuntu-latest
-    if: ${{ failure() }}
+    if: ${{ false }} # Disabled due to requiring Slack integration
     steps:
       - name: Notify Slack on failure
         uses: rtCamp/action-slack-notify@v2.2.0

.github/workflows/test.yml

@@ -142,6 +142,7 @@ jobs:
           path: ./tests/e2e-profile.out
 
   repo-analysis:
+    if: ${{ false }} # Disabled due to requiring SonarCloud integration
     runs-on: ubuntu-latest
     needs: [tests, test-integration, test-e2e]
     steps:
@@ -181,7 +182,7 @@ jobs:
           name: "${{ github.sha }}-e2e-coverage"
         continue-on-error: true
       - name: sonarcloud
-        if: env.GIT_DIFF
+        if: env.GIT_DIFF && secrets.SONAR_TOKEN
         uses: SonarSource/sonarcloud-github-action@master
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -272,7 +273,7 @@ jobs:
           cd simapp
           go test -mod=readonly -timeout 30m -tags='app_v1 norace ledger test_ledger_mock rocksdb_build' ./...
       - name: sonarcloud
-        if: env.GIT_DIFF
+        if: env.GIT_DIFF && secrets.SONAR_TOKEN
         uses: SonarSource/sonarcloud-github-action@master
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

README.md

@@ -1,3 +1,43 @@
+# dYdX Fork of CosmosSDK
+
+This is a lightweight fork of CosmosSDK. The current version of the forked code resides on the [default branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches#about-the-default-branch).
+
+## Making Changes to the Fork
+
+1. Open a PR against the current default branch (i.e. `dydx-fork-v0.47.1`).
+2. Get approval, and merge. *DO NOT SQUASH AND MERGE. PLEASE REBASE AND MERGE*
+3. After merging, update the `v4` repository's `go.mod`, and `go.sum` files with your merged `$COMMIT_HASH`.
+4. (In `dydxprotocol/v4`) `go mod edit -replace github.com/cosmos/cosmos-sdk=github.com/dydxprotocol/cosmos-sdk@$COMMIT_HASH`
+5. (In `dydxprotocol/v4`) `go mod tidy`
+6. (In `dydxprotocol/v4`) update package references in `mocks/Makefile`. See [here](https://github.com/dydxprotocol/v4/pull/848) for an example.
+7. Open a PR in `dydxprotocol/v4` to bump the version of the fork.
+
+## Fork maintenance
+
+We'd like to keep the `main` branch up to date with `cosmos/cosmos-sdk`. You can utilize GitHub's [sync fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork) button to accomplish this. ⚠️ Please only use this on the `main` branch, not on the fork branches as it will discard our commits.⚠️
+
+Before these steps, please set the `upstream` remote to be the `cosmos/cosmos-sdk` repo. If you use http auth, you will have to switch the upstream url to be http.
+`git remote add upstream git@github.com:cosmos/cosmos-sdk.git`
+
+Note that this doesn't pull in upstream tags, so in order to do this follow these steps:
+1. `git fetch upstream`
+2. `git push --tags`
+
+## Updating CosmosSDK to new versions
+
+When a new version of CosmosSDK is published, we may want to adopt the changes in our fork. This process can be somewhat tedious, but below are the recommended steps to accomplish this.
+
+1. Ensure the `main` branch and all tags are up to date by following the steps above in "Fork maintenance".
+2. Create a new branch off the desired CosmosSDK commit using tags. `git checkout -b dydx-fork-$VERSION <CosmosSDK repo's tag name>`. The new branch should be named something like `dydx-fork-$VERSION` where `$VERSION` is the version of CosmosSDK being forked (should match the CosmosSDK repo's tag name). i.e. `dydx-fork-v0.47.1`.
+3. Push the new branch (i.e `dydx-fork-v0.47.1`).
+4. Off of this new branch, create a new branch. (i.e `totoro/dydxCommits`)
+5. Cherry-pick each dydx-created commit from the current default branch, in order, on to the new `dydx-fork-$VERSION` branch (note: you may want to consider creating multiple PRs for this process if there are difficulties or merge conflicts). For example, `git cherry-pick <commit hash>`. You can verify the first commit by seeing the most recent commit sha for the `$VERSION` (i.e `v0.47.1`) tag on the `cosmos/cosmos-sdk` repo, and taking the next commit from that sha.
+6. Open a PR to merge the second branch (`totoro/dydxCommits`) into the first (`dydx-fork-v0.47.1`). Get approval, and merge. *DO NOT SQUASH AND MERGE. PLEASE REBASE AND MERGE*
+7. Update `dydxprotocol/v4` by following the steps in "Making Changes to the fork" above.
+8. Set `dydx-fork-$VERSION` as the [default branch](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/changing-the-default-branch) in this repository.
+
+Note that the github CI workflows need permissioning. If they fail with a permissioning error, i.e `Resource not accessible through integration`, a Github admin will need to whitelist the workflow.
+
 <div align="center">
   <h1> Cosmos SDK </h1>
 </div>

baseapp/abci.go

@@ -35,6 +35,9 @@ const (
 // InitChain implements the ABCI interface. It runs the initialization logic
 // directly on the CommitMultiStore.
 func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if req.ChainId != app.chainID {
 		panic(fmt.Sprintf("invalid chain-id on InitChain; expected: %s, got: %s", app.chainID, req.ChainId))
 	}
@@ -68,6 +71,16 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 		app.StoreConsensusParams(app.deliverState.ctx, req.ConsensusParams)
 	}
 
+	defer func() {
+		// InitChain represents the state of the application BEFORE the first block,
+		// i.e. the genesis block. This means that when processing the app's InitChain
+		// handler, the block height is zero by default. However, after Commit is called
+		// the height needs to reflect the true block height.
+		initHeader.Height = req.InitialHeight
+		app.checkState.ctx = app.checkState.ctx.WithBlockHeader(initHeader)
+		app.deliverState.ctx = app.deliverState.ctx.WithBlockHeader(initHeader)
+	}()
+
 	if app.initChainer == nil {
 		return
 	}
@@ -110,8 +123,8 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 		appHash = emptyHash[:]
 	}
 
-	// NOTE: We don't commit, but BeginBlock for block `initial_height` starts from this
-	// deliverState.
+	// NOTE: We don't commit, but BeginBlock for block InitialHeight starts from
+	// this deliverState.
 	return abci.ResponseInitChain{
 		ConsensusParams: res.ConsensusParams,
 		Validators:      res.Validators,
@@ -121,6 +134,9 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 
 // Info implements the ABCI interface.
 func (app *BaseApp) Info(req abci.RequestInfo) abci.ResponseInfo {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	lastCommitID := app.cms.LastCommitID()
 
 	return abci.ResponseInfo{
@@ -152,6 +168,9 @@ func (app *BaseApp) FilterPeerByID(info string) abci.ResponseQuery {
 
 // BeginBlock implements the ABCI application interface.
 func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if req.Header.ChainID != app.chainID {
 		panic(fmt.Sprintf("invalid chain-id on BeginBlock; expected: %s, got: %s", app.chainID, req.Header.ChainID))
 	}
@@ -211,6 +230,9 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 
 // EndBlock implements the ABCI interface.
 func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.deliverState.ms.TracingEnabled() {
 		app.deliverState.ms = app.deliverState.ms.SetTracingContext(nil).(sdk.CacheMultiStore)
 	}
@@ -248,6 +270,9 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
 // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md
 // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md
 func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.ResponsePrepareProposal) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.prepareProposal == nil {
 		panic("PrepareProposal method not set")
 	}
@@ -305,6 +330,9 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
 // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md
 // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md
 func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.ResponseProcessProposal) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.processProposal == nil {
 		panic("app.ProcessProposal is not set")
 	}
@@ -367,7 +395,7 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 		panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type))
 	}
 
-	gInfo, result, anteEvents, priority, err := app.runTx(mode, req.Tx)
+	gInfo, result, anteEvents, priority, err := app.runCheckTxConcurrently(mode, req.Tx)
 	if err != nil {
 		return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace)
 	}
@@ -388,6 +416,18 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 // Regardless of tx execution outcome, the ResponseDeliverTx will contain relevant
 // gas execution context.
 func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliverTx) {
+	return app.DeliverTxShouldLock(req, true)
+}
+
+// DeliverTxShouldLock enables control of whether the lock should be acquired. The golang mutex
+// is not reentrant so we enable conditional locking to prevent deadlock since cosmos-sdk/x/genutil.InitGenesis
+// invokes DeliverTx from the ABCI++ InitGenesis method which already holds the lock.
+func (app *BaseApp) DeliverTxShouldLock(req abci.RequestDeliverTx, needsLock bool) (res abci.ResponseDeliverTx) {
+	if needsLock {
+		app.mtx.Lock()
+		defer app.mtx.Unlock()
+	}
+
 	gInfo := sdk.GasInfo{}
 	resultStr := "successful"
 
@@ -429,6 +469,9 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv
 // against that height and gracefully halt if it matches the latest committed
 // height.
 func (app *BaseApp) Commit() abci.ResponseCommit {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	header := app.deliverState.ctx.BlockHeader()
 	retainHeight := app.GetBlockRetentionHeight(header.Height)
 
@@ -466,6 +509,10 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
 	// empty/reset the deliver state
 	app.deliverState = nil
 
+	if app.commiter != nil {
+		app.commiter(app.checkState.ctx)
+	}
+
 	var halt bool
 
 	switch {
@@ -514,6 +561,9 @@ func (app *BaseApp) halt() {
 // Query implements the ABCI interface. It delegates to CommitMultiStore if it
 // implements Queryable.
 func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	// Add panic recovery for all queries.
 	// ref: https://github.com/cosmos/cosmos-sdk/pull/8039
 	defer func() {
@@ -563,6 +613,9 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 
 // ListSnapshots implements the ABCI interface. It delegates to app.snapshotManager if set.
 func (app *BaseApp) ListSnapshots(req abci.RequestListSnapshots) abci.ResponseListSnapshots {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	resp := abci.ResponseListSnapshots{Snapshots: []*abci.Snapshot{}}
 	if app.snapshotManager == nil {
 		return resp
@@ -588,6 +641,9 @@ func (app *BaseApp) ListSnapshots(req abci.RequestListSnapshots) abci.ResponseLi
 
 // LoadSnapshotChunk implements the ABCI interface. It delegates to app.snapshotManager if set.
 func (app *BaseApp) LoadSnapshotChunk(req abci.RequestLoadSnapshotChunk) abci.ResponseLoadSnapshotChunk {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.snapshotManager == nil {
 		return abci.ResponseLoadSnapshotChunk{}
 	}
@@ -607,6 +663,9 @@ func (app *BaseApp) LoadSnapshotChunk(req abci.RequestLoadSnapshotChunk) abci.Re
 
 // OfferSnapshot implements the ABCI interface. It delegates to app.snapshotManager if set.
 func (app *BaseApp) OfferSnapshot(req abci.RequestOfferSnapshot) abci.ResponseOfferSnapshot {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.snapshotManager == nil {
 		app.logger.Error("snapshot manager not configured")
 		return abci.ResponseOfferSnapshot{Result: abci.ResponseOfferSnapshot_ABORT}
@@ -656,6 +715,9 @@ func (app *BaseApp) OfferSnapshot(req abci.RequestOfferSnapshot) abci.ResponseOf
 
 // ApplySnapshotChunk implements the ABCI interface. It delegates to app.snapshotManager if set.
 func (app *BaseApp) ApplySnapshotChunk(req abci.RequestApplySnapshotChunk) abci.ResponseApplySnapshotChunk {
+	app.mtx.Lock()
+	defer app.mtx.Unlock()
+
 	if app.snapshotManager == nil {
 		app.logger.Error("snapshot manager not configured")
 		return abci.ResponseApplySnapshotChunk{Result: abci.ResponseApplySnapshotChunk_ABORT}

baseapp/abci_test.go

@@ -7,9 +7,9 @@ import (
 	"strings"
 	"testing"
 
-	dbm "github.com/cometbft/cometbft-db"
 	abci "github.com/cometbft/cometbft/abci/types"
 	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	dbm "github.com/cometbft/cometbft-db"
 	"github.com/cosmos/gogoproto/jsonpb"
 	"github.com/stretchr/testify/require"
 
@@ -39,6 +39,21 @@ func TestABCI_Info(t *testing.T) {
 	require.Equal(t, suite.baseApp.AppVersion(), res.AppVersion)
 }
 
+func TestABCI_First_block_Height(t *testing.T) {
+	suite := NewBaseAppSuite(t, baseapp.SetChainID("test-chain-id"))
+	app := suite.baseApp
+
+	app.InitChain(abci.RequestInitChain{
+		ChainId:         "test-chain-id",
+		ConsensusParams: &cmtproto.ConsensusParams{Block: &cmtproto.BlockParams{MaxGas: 5000000}},
+		InitialHeight:   1,
+	})
+	_ = app.Commit()
+
+	ctx := app.GetContextForCheckTx(nil)
+	require.Equal(t, int64(1), ctx.BlockHeight())
+}
+
 func TestABCI_InitChain(t *testing.T) {
 	name := t.Name()
 	db := dbm.NewMemDB()
@@ -596,6 +611,33 @@ func TestABCI_EndBlock(t *testing.T) {
 	require.Equal(t, cp.Block.MaxGas, res.ConsensusParamUpdates.Block.MaxGas)
 }
 
+func TestBaseApp_Commit(t *testing.T) {
+	db := dbm.NewMemDB()
+	name := t.Name()
+	logger := defaultLogger()
+
+	cp := &tmproto.ConsensusParams{
+		Block: &tmproto.BlockParams{
+			MaxGas: 5000000,
+		},
+	}
+
+	app := baseapp.NewBaseApp(name, logger, db, nil)
+	app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
+	app.InitChain(abci.RequestInitChain{
+		ConsensusParams: cp,
+	})
+
+	wasCommiterCalled := false
+	app.SetCommiter(func(ctx sdk.Context) {
+		wasCommiterCalled = true
+	})
+	app.Seal()
+
+	app.Commit()
+	require.Equal(t, true, wasCommiterCalled)
+}
+
 func TestABCI_CheckTx(t *testing.T) {
 	// This ante handler reads the key and checks that the value matches the
 	// current counter. This ensures changes to the KVStore persist across
@@ -1320,6 +1362,28 @@ func TestABCI_GetBlockRetentionHeight(t *testing.T) {
 	}
 }
 
+// Verifies that the Commiter is called with the checkState.
+func TestCommiterCalledWithCheckState(t *testing.T) {
+	t.Parallel()
+
+	logger := defaultLogger()
+	db := dbm.NewMemDB()
+	name := t.Name()
+	app := baseapp.NewBaseApp(name, logger, db, nil)
+
+	wasCommiterCalled := false
+	app.SetCommiter(func(ctx sdk.Context) {
+		// Make sure context is for next block
+		require.Equal(t, true, ctx.IsCheckTx())
+		wasCommiterCalled = true
+	})
+
+	app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
+	app.Commit()
+
+	require.Equal(t, true, wasCommiterCalled)
+}
+
 func TestABCI_Proposal_HappyPath(t *testing.T) {
 	anteKey := []byte("ante-key")
 	pool := mempool.NewSenderNonceMempool()