Skip to content

Fix broken CI and unit tests#1

Merged
prettymuchbryce merged 7 commits intodydx-fork-v0.37.0-rc2from
prettymuchbryce/enable-ci
Feb 2, 2023
Merged

Fix broken CI and unit tests#1
prettymuchbryce merged 7 commits intodydx-fork-v0.37.0-rc2from
prettymuchbryce/enable-ci

Conversation

@prettymuchbryce
Copy link

@prettymuchbryce prettymuchbryce commented Feb 2, 2023

Fixes unit and end-to-end tests broken by our Tendermint/CometBFT fork.

  • Fixes TestEndBlockValidatorUpdatesResultingInEmptySet to account for the new mempool calls added in ApplyBlock.
  • Fixes nil pointer exceptions happening in cases where cosmosTx.Body was nil.
  • Including cosmos-sdk in this repository in this commit modified a lot of dependencies due to the way that Go dependency resolution works (as Tendermint and CosmosSDK share a number of dependencies and Go uses MVS). This was breaking some end-to-end tests due to a change in tm-db from from v0.6.6 to v0.6.7. Specifically tm-db includes rocksdb as a dependency and v0.6.7 seems to change rocksdb from tecbot/gorocksdb to cosmos/gorocksdb which seems to be a breaking change, incompatible with the way rocksdb is being installed in the end-to-end tests in this repository. As a solution here I added a replacement direction to use tm-db at v0.6.6, which is the version used upstream.
  • Resets KeepInvalidTxsInCache to false in DefaultMempoolConfig, which was causing unit tests to fail. This value should was modified in this commit, but instead should be modified in the configuration, so I did that here in v4 instead.

@prettymuchbryce prettymuchbryce marked this pull request as ready for review February 2, 2023 14:27
@prettymuchbryce prettymuchbryce changed the title CI Testing Fix broken CI and unit tests Feb 2, 2023
}

if len(cosmosTx.Body.Messages) == 1 &&
if cosmosTx.Body != nil &&
Copy link
Author

@prettymuchbryce prettymuchbryce Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was potentially a bug that could have caused panics.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch! in what case can the body be empty? is it possible to send an empty body tx?

Copy link
Author

@prettymuchbryce prettymuchbryce Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if it's possible in practice, but this was happening in unit tests. It seems best to be defensive here.

{PubKey: vp, Power: 0},
}

// dYdX fork: Apply block should lock/unlock the mempool.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for my own understanding, could you point me to where Lock/Unlock/FlushAppConn were added?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's this commit.

}

if len(cosmosTx.Body.Messages) == 1 &&
if cosmosTx.Body != nil &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch! in what case can the body be empty? is it possible to send an empty body tx?

replace github.com/cosmos/cosmos-sdk v0.47.0-alpha2 => github.com/dydxprotocol/cosmos-sdk v0.47.0-alpha2-dydx-fork
replace (
github.com/cosmos/cosmos-sdk v0.47.0-alpha2 => github.com/dydxprotocol/cosmos-sdk v0.47.0-alpha2-dydx-fork
github.com/tendermint/tm-db => github.com/tendermint/tm-db v0.6.6
Copy link

@ttl33 ttl33 Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above, there's github.com/tendermint/tm-db v0.6.7 which makes me believe that CometBFT intends to use v0.6.7. However, we are overriding that to a prev version here. Why is this the case?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This question is answered in the PR description

@prettymuchbryce prettymuchbryce merged commit 677d636 into dydx-fork-v0.37.0-rc2 Feb 2, 2023
@prettymuchbryce prettymuchbryce deleted the prettymuchbryce/enable-ci branch February 2, 2023 23:15
lcwik pushed a commit that referenced this pull request Apr 26, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
lcwik pushed a commit that referenced this pull request Apr 27, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jun 30, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jul 2, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jul 2, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jul 2, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jul 3, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
roy-dydx pushed a commit that referenced this pull request Jul 11, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
lcwik added a commit that referenced this pull request Aug 30, 2023
lcwik added a commit that referenced this pull request Aug 30, 2023
lcwik added a commit that referenced this pull request Aug 30, 2023
matthewdowney pushed a commit to matthewdowney/cometbft that referenced this pull request Aug 31, 2023
* Test commit

* empty commit to trigger build

* empty commit to trigger build

* Fix TestEndBlockValidatorUpdatesResultingInEmptySet

* Fix test failures

* Add replace using tm-db v0.6.6

* Remove test comment
pthmas pushed a commit that referenced this pull request Aug 28, 2025
due to sec vuln

Vulnerability #1: GO-2025-3420
Sensitive headers incorrectly sent after cross-domain redirect in
net/http
  More info: https://pkg.go.dev/vuln/GO-2025-3420
  Standard library
    Found in: net/http@go1.23.1
    Fixed in: net/http@go1.23.5
    Example traces found:
Error: #1: rpc/jsonrpc/client/http_json_client.go:231:34:
client.Client.Call calls http.Client.Do
Error: #2: libs/cli/setup.go:89:26: cli.Executor.Execute calls
cobra.Command.Execute, which eventually calls http.Client.Get
Error: #3: cmd/cometbft/commands/debug/util.go:70:23: debug.dumpProfile
calls http.Get

Vulnerability #2: GO-2025-3373
Usage of IPv6 zone IDs can bypass URI name constraints in crypto/x509
  More info: https://pkg.go.dev/vuln/GO-2025-3373
  Standard library
    Found in: crypto/x509@go1.23.1
    Fixed in: crypto/x509@go1.23.5
    Example traces found:
Error: #1: abci/tutorials/abci-v2-forum-app/model/db.go:143:20:
model.DB.Close calls badger.DB.Close, which eventually calls
x509.CertPool.AppendCertsFromPEM
Error: #2: internal/autofile/group.go:468:30: autofile.GroupReader.Read
calls bufio.Reader.Read, which eventually calls x509.Certificate.Verify
Error: #3: rpc/jsonrpc/client/ws_client.go:290:29: client.WSClient.dial
calls websocket.Dialer.Dial, which eventually calls
x509.Certificate.VerifyHostname
Error: #4: light/errors.go:483:84: light.errBadWitness.Error calls
x509.HostnameError.Error
Error: #5: rpc/jsonrpc/server/http_server.go:166:19:
server.ServeTLSWithShutdown calls http.Server.ServeTLS, which eventually
calls x509.ParseCertificate
Error: #6: rpc/jsonrpc/server/http_server.go:166:19:
server.ServeTLSWithShutdown calls http.Server.ServeTLS, which eventually
calls x509.ParseECPrivateKey
Error: #7: rpc/jsonrpc/server/http_server.go:166:19:
server.ServeTLSWithShutdown calls http.Server.ServeTLS, which eventually
calls x509.ParsePKCS1PrivateKey
Error: #8: rpc/jsonrpc/server/http_server.go:166:19:
server.ServeTLSWithShutdown calls http.Server.ServeTLS, which eventually
calls x509.ParsePKCS8PrivateKey
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants