Add dYdX commits to cosmos v0.47.1 (#11)

* Implement Commiter

* Update README with fork instructions

* Enable CI workflows and fix failures (#4)

* Fix lint/gocritic error

* Disable some workflow jobs

* Add tests for Commiter change (#5)

* Add additional upgrade step to README

* add precommit callback (#7)

* Revert "add precommit callback (#7)" (#10)

This reverts commit 0e85c49.

* cosmos-sdk upgrade README changes

---------

Co-authored-by: Bryce Neal <bryce@dydx.exchange>
Co-authored-by: Bryce Neal <brycedneal@gmail.com>
Co-authored-by: dydxwill <119354122+dydxwill@users.noreply.github.com>

.github/workflows/sims.yml

@@ -84,7 +84,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
@@ -112,7 +112,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

@@ -138,6 +138,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:

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.0-alpha2`).
+2. Get approval, 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.0-alpha2`.
+3. Push the new branch (i.e `dydx-fork-v0.47.0-alpha2`).
+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.0-alpha2`) 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.0-alpha2`). Get approval, 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

@@ -488,6 +488,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 {

baseapp/abci_test.go

@@ -598,6 +598,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 := log.NewTestLogger(t)
+
+	cp := &cmtproto.ConsensusParams{
+		Block: &cmtproto.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
@@ -1322,6 +1349,28 @@ func TestABCI_GetBlockRetentionHeight(t *testing.T) {
 	}
 }
 
+// Verifies that the Commiter is called with the checkState.
+func TestCommiterCalledWithCheckState(t *testing.T) {
+	t.Parallel()
+
+	logger := log.NewTestLogger(t)
+	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: cmtproto.Header{Height: 1}})
+	app.Commit()
+
+	require.Equal(t, true, wasCommiterCalled)
+}
+
 func TestABCI_Proposal_HappyPath(t *testing.T) {
 	anteKey := []byte("ante-key")
 	pool := mempool.NewSenderNonceMempool()

baseapp/baseapp.go

@@ -70,6 +70,7 @@ type BaseApp struct { //nolint: maligned
 	processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal
 	prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal
 	endBlocker      sdk.EndBlocker             // logic to run after all txs, and to determine valset changes
+	commiter        sdk.Commiter               // logic to run during commit
 	addrPeerFilter  sdk.PeerFilter             // filter peers by address and port
 	idPeerFilter    sdk.PeerFilter             // filter peers by node ID
 	fauxMerkleMode  bool                       // if true, IAVL MountStores uses MountStoresDB for simulation speed.

baseapp/baseapp_test.go

@@ -388,6 +388,9 @@ func TestBaseAppOptionSeal(t *testing.T) {
 	require.Panics(t, func() {
 		suite.baseApp.SetEndBlocker(nil)
 	})
+	require.Panics(t, func() {
+		suite.baseApp.SetCommiter(nil)
+	})
 	require.Panics(t, func() {
 		suite.baseApp.SetAnteHandler(nil)
 	})

baseapp/options.go

@@ -167,6 +167,14 @@ func (app *BaseApp) SetEndBlocker(endBlocker sdk.EndBlocker) {
 	app.endBlocker = endBlocker
 }
 
+func (app *BaseApp) SetCommiter(commiter sdk.Commiter) {
+	if app.sealed {
+		panic("SetCommiter() on sealed BaseApp")
+	}
+
+	app.commiter = commiter
+}
+
 func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) {
 	if app.sealed {
 		panic("SetAnteHandler() on sealed BaseApp")

telemetry/wrapper.go

@@ -11,6 +11,7 @@ const (
 	MetricKeyBeginBlocker = "begin_blocker"
 	MetricKeyEndBlocker   = "end_blocker"
 	MetricLabelNameModule = "module"
+	MetricKeyCommit       = "commit"
 )
 
 // NewLabel creates a new instance of Label with name and value

types/abci.go

@@ -19,6 +19,10 @@ type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) (abci.ResponseBe
 // e.g. BFT timestamps rather than block height for any periodic EndBlock logic
 type EndBlocker func(ctx Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error)
 
+// Commiter runs code during commit after the block number has been incremented and the `checkState` has been
+// branched for the new block.
+type Commiter func(ctx Context)
+
 // PeerFilter responds to p2p filtering queries from Tendermint
 type PeerFilter func(info string) abci.ResponseQuery
 

types/module/module.go

@@ -212,6 +212,12 @@ type EndBlockAppModule interface {
 	EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate
 }
 
+// CommitAppModule is an extension interface that contains information about the AppModule and Commit.
+type CommitAppModule interface {
+	AppModule
+	Commit(sdk.Context)
+}
+
 // GenesisOnlyAppModule is an AppModule that only has import/export functionality
 type GenesisOnlyAppModule struct {
 	AppModuleGenesis
@@ -258,6 +264,7 @@ type Manager struct {
 	OrderExportGenesis []string
 	OrderBeginBlockers []string
 	OrderEndBlockers   []string
+	OrderCommiters     []string
 	OrderMigrations    []string
 }
 
@@ -275,6 +282,7 @@ func NewManager(modules ...AppModule) *Manager {
 		OrderInitGenesis:   modulesStr,
 		OrderExportGenesis: modulesStr,
 		OrderBeginBlockers: modulesStr,
+		OrderCommiters:     modulesStr,
 		OrderEndBlockers:   modulesStr,
 	}
 }
@@ -340,6 +348,11 @@ func (m *Manager) SetOrderBeginBlockers(moduleNames ...string) {
 	m.OrderBeginBlockers = moduleNames
 }
 
+// SetOrderCommiters sets the order of set commiter calls
+func (m *Manager) SetOrderCommiters(moduleNames ...string) {
+	m.OrderCommiters = moduleNames
+}
+
 // SetOrderEndBlockers sets the order of set end-blocker calls
 func (m *Manager) SetOrderEndBlockers(moduleNames ...string) {
 	m.assertNoForgottenModules("SetOrderEndBlockers", moduleNames,
@@ -706,6 +719,17 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) (abci.Resp
 	}, nil
 }
 
+// Commit performs commit functionality for all modules.
+func (m *Manager) Commit(ctx sdk.Context) {
+	for _, moduleName := range m.OrderCommiters {
+		module, ok := m.Modules[moduleName].(CommitAppModule)
+		if !ok {
+			continue
+		}
+		module.Commit(ctx)
+	}
+}
+
 // GetVersionMap gets consensus version from all modules
 func (m *Manager) GetVersionMap() VersionMap {
 	vermap := make(VersionMap)