Skip to content

Commit

Permalink
fix: Port state sync fixes to Cosmos SDK v0.46.10 [DEV-2290] (#156)
Browse files Browse the repository at this point in the history
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: #XXXX

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is
not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR
Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building
modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration
[tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go
code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable
and please add
your handle next to the items reviewed if you only reviewed selected
items.*

I have...

- [ ] confirmed the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
Andrew Nikitin committed Feb 28, 2023
2 parents 9fb19e6 + 81a1ffa commit acea3e4
Show file tree
Hide file tree
Showing 33 changed files with 322 additions and 202 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
uses: rtCamp/action-slack-notify@v2.2.0
env:
SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }}
SLACK_CHANNEL: cosmos-sdk
SLACK_CHANNEL: cosmos-tech
SLACK_USERNAME: Cosmos SDK Release Bot
SLACK_ICON: https://avatars.githubusercontent.com/t/5997665?size=64
SLACK_COLOR: good
Expand Down
29 changes: 28 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,34 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

## [v0.46.8](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.8) - 2022-01-23
## [v0.46.10](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.10) - 2022-02-16

### Improvements

* (cli) [#14953](https://github.com/cosmos/cosmos-sdk/pull/14953) Enable profiling block replay during abci handshake with `--cpu-profile`.

## [v0.46.9](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.9) - 2022-02-07

### Improvements

* (deps) [#14846](https://github.com/cosmos/cosmos-sdk/pull/14846) Bump btcd.
* (deps) Bump Tendermint version to [v0.34.26](https://github.com/informalsystems/tendermint/releases/tag/v0.34.26).
* (store) [#14189](https://github.com/cosmos/cosmos-sdk/pull/14189) Add config `iavl-lazy-loading` to enable lazy loading of iavl store, to improve start up time of archive nodes, add method `SetLazyLoading` to `CommitMultiStore` interface.
* A new field has been added to the app.toml. This alllows nodes with larger databases to startup quicker

```toml
# IAVLLazyLoading enable/disable the lazy loading of iavl store.
# Default is false.
iavl-lazy-loading = ""
```

### Bug Fixes

* (cli) [#14919](https://github.com/cosmos/cosmos-sdk/pull/#14919) Fix never assigned error when write validators.
* (store) [#14798](https://github.com/cosmos/cosmos-sdk/pull/14798) Copy btree to avoid the problem of modify while iteration.
* (cli) [#14799](https://github.com/cosmos/cosmos-sdk/pull/14799) Fix Evidence CLI query flag parsing (backport #13458)

## [v0.46.8](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.8) - 2023-01-23

### Improvements

Expand Down
18 changes: 3 additions & 15 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
# Cosmos SDK v0.46.8 Release Notes
# Cosmos SDK v0.46.10 Release Notes

This release introduces bug fixes and improvements. Notably, the SDK have now switched to Informal Systems' Tendermint fork.
Their fork has no changes compared to the upstream Tendermint, but it is now [maintained by Informal Systems](https://twitter.com/informalinc/status/1613580954383040512). Chains are invited to do the same.

Moreover, this release contains a store fix. The changes have been tested against a v0.46.x chain mainnet with no issues. However, there is a low probability of an edge case happening. Hence, it is recommended to do a **coordinated upgrade** to avoid any issues.
This release improves CPU profiling when using the `--cpu-profile` flag, and fixes a possible way to DoS a node.

Please see the [CHANGELOG](https://github.com/cosmos/cosmos-sdk/blob/release/v0.46.x/CHANGELOG.md) for an exhaustive list of changes.

Full Commit History: https://github.com/cosmos/cosmos-sdk/compare/v0.46.7...v0.46.8

**NOTE**: The changes mentioned in `v0.46.3` are no longer required. The following replace directive can be removed from the chains.

```go
# Can be deleted from go.mod
replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v0.8.0
```

Instead, `github.com/confio/ics23/go` must be **bumped to `v0.9.0`**.
Full Commit History: https://github.com/cosmos/cosmos-sdk/compare/v0.46.9...v0.46.10
4 changes: 4 additions & 0 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,10 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
telemetry.IncrCounter(1, "query", req.Path)
defer telemetry.MeasureSince(time.Now(), req.Path)

if req.Path == "/cosmos.tx.v1beta1.Service/BroadcastTx" {
return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "can't route a broadcast tx message"), app.trace)
}

// handle gRPC routes first rather than calling splitPath because '/' characters
// are used as part of gRPC paths
if grpcHandler := app.grpcQueryRouter.Route(req.Path); grpcHandler != nil {
Expand Down
5 changes: 5 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ func SetIAVLDisableFastNode(disable bool) func(*BaseApp) {
return func(bapp *BaseApp) { bapp.cms.SetIAVLDisableFastNode(disable) }
}

// SetIAVLLazyLoading enables/disables lazy loading of the IAVL store.
func SetIAVLLazyLoading(lazyLoading bool) func(*BaseApp) {
return func(bapp *BaseApp) { bapp.cms.SetLazyLoading(lazyLoading) }
}

// SetInterBlockCache provides a BaseApp option function that sets the
// inter-block cache.
func SetInterBlockCache(cache sdk.MultiStorePersistentCache) func(*BaseApp) {
Expand Down
4 changes: 2 additions & 2 deletions crypto/hd/hdpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"strconv"
"strings"

"github.com/btcsuite/btcd/btcec"
"github.com/btcsuite/btcd/btcec/v2"
)

// NewParams creates a BIP 44 parameter object from the params:
Expand Down Expand Up @@ -225,7 +225,7 @@ func derivePrivateKey(privKeyBytes [32]byte, chainCode [32]byte, index uint32, h
data = append([]byte{byte(0)}, privKeyBytes[:]...)
} else {
// this can't return an error:
_, ecPub := btcec.PrivKeyFromBytes(btcec.S256(), privKeyBytes[:])
_, ecPub := btcec.PrivKeyFromBytes(privKeyBytes[:])
pubkeyBytes := ecPub.SerializeCompressed()
data = pubkeyBytes

Expand Down
4 changes: 2 additions & 2 deletions crypto/keys/secp256k1/secp256k1.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"io"
"math/big"

secp256k1 "github.com/btcsuite/btcd/btcec"
secp256k1 "github.com/btcsuite/btcd/btcec/v2"
"github.com/tendermint/tendermint/crypto"
"golang.org/x/crypto/ripemd160" // nolint: staticcheck // necessary for Bitcoin address format

Expand Down Expand Up @@ -37,7 +37,7 @@ func (privKey *PrivKey) Bytes() []byte {
// PubKey performs the point-scalar multiplication from the privKey on the
// generator point to get the pubkey.
func (privKey *PrivKey) PubKey() cryptotypes.PubKey {
_, pubkeyObject := secp256k1.PrivKeyFromBytes(secp256k1.S256(), privKey.Key)
_, pubkeyObject := secp256k1.PrivKeyFromBytes(privKey.Key)
pk := pubkeyObject.SerializeCompressed()
return &PubKey{Key: pk}
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/secp256k1/secp256k1_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"math/big"
"testing"

btcSecp256k1 "github.com/btcsuite/btcd/btcec"
btcSecp256k1 "github.com/btcsuite/btcd/btcec/v2"
"github.com/stretchr/testify/require"
)

Expand Down
52 changes: 20 additions & 32 deletions crypto/keys/secp256k1/secp256k1_nocgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,22 @@
package secp256k1

import (
"math/big"

secp256k1 "github.com/btcsuite/btcd/btcec"
secp256k1 "github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/ecdsa"

"github.com/tendermint/tendermint/crypto"
)

// used to reject malleable signatures
// see:
// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93
// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39
var secp256k1halfN = new(big.Int).Rsh(secp256k1.S256().N, 1)

// Sign creates an ECDSA signature on curve Secp256k1, using SHA256 on the msg.
// The returned signature will be of the form R || S (in lower-S form).
func (privKey *PrivKey) Sign(msg []byte) ([]byte, error) {
priv, _ := secp256k1.PrivKeyFromBytes(secp256k1.S256(), privKey.Key)
sig, err := priv.Sign(crypto.Sha256(msg))
priv, _ := secp256k1.PrivKeyFromBytes(privKey.Key)
sig, err := ecdsa.SignCompact(priv, crypto.Sha256(msg), false)
if err != nil {
return nil, err
}
sigBytes := serializeSig(sig)
return sigBytes, nil
// remove the first byte which is compactSigRecoveryCode
return sig[1:], nil
}

// VerifyBytes verifies a signature of the form R || S.
Expand All @@ -35,37 +28,32 @@ func (pubKey *PubKey) VerifySignature(msg []byte, sigStr []byte) bool {
if len(sigStr) != 64 {
return false
}
pub, err := secp256k1.ParsePubKey(pubKey.Key, secp256k1.S256())
pub, err := secp256k1.ParsePubKey(pubKey.Key)
if err != nil {
return false
}
// parse the signature:
signature := signatureFromBytes(sigStr)
// Reject malleable signatures. libsecp256k1 does this check but btcec doesn't.
// see: https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93
if signature.S.Cmp(secp256k1halfN) > 0 {
// Serialize() would negate S value if it is over half order.
// Hence, if the signature is different after Serialize() if should be rejected.
modifiedSignature, parseErr := ecdsa.ParseDERSignature(signature.Serialize())
if parseErr != nil {
return false
}
if !signature.IsEqual(modifiedSignature) {
return false
}
return signature.Verify(crypto.Sha256(msg), pub)
}

// Read Signature struct from R || S. Caller needs to ensure
// that len(sigStr) == 64.
func signatureFromBytes(sigStr []byte) *secp256k1.Signature {
return &secp256k1.Signature{
R: new(big.Int).SetBytes(sigStr[:32]),
S: new(big.Int).SetBytes(sigStr[32:64]),
}
}

// Serialize signature to R || S.
// R, S are padded to 32 bytes respectively.
func serializeSig(sig *secp256k1.Signature) []byte {
rBytes := sig.R.Bytes()
sBytes := sig.S.Bytes()
sigBytes := make([]byte, 64)
// 0 pad the byte arrays from the left if they aren't big enough.
copy(sigBytes[32-len(rBytes):32], rBytes)
copy(sigBytes[64-len(sBytes):64], sBytes)
return sigBytes
func signatureFromBytes(sigStr []byte) *ecdsa.Signature {
var r secp256k1.ModNScalar
r.SetByteSlice(sigStr[:32])
var s secp256k1.ModNScalar
s.SetByteSlice(sigStr[32:64])
return ecdsa.NewSignature(&r, &s)
}
23 changes: 16 additions & 7 deletions crypto/keys/secp256k1/secp256k1_nocgo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package secp256k1
import (
"testing"

secp256k1 "github.com/btcsuite/btcd/btcec"
secp256k1 "github.com/btcsuite/btcd/btcec/v2"
"github.com/stretchr/testify/require"
)

Expand All @@ -19,20 +19,29 @@ func TestSignatureVerificationAndRejectUpperS(t *testing.T) {
priv := GenPrivKey()
sigStr, err := priv.Sign(msg)
require.NoError(t, err)
sig := signatureFromBytes(sigStr)
require.False(t, sig.S.Cmp(secp256k1halfN) > 0)
var r secp256k1.ModNScalar
r.SetByteSlice(sigStr[:32])
var s secp256k1.ModNScalar
s.SetByteSlice(sigStr[32:64])
require.False(t, s.IsOverHalfOrder())

pub := priv.PubKey()
require.True(t, pub.VerifySignature(msg, sigStr))

// malleate:
sig.S.Sub(secp256k1.S256().CurveParams.N, sig.S)
require.True(t, sig.S.Cmp(secp256k1halfN) > 0)
malSigStr := serializeSig(sig)
var S256 secp256k1.ModNScalar
S256.SetByteSlice(secp256k1.S256().N.Bytes())
s.Negate().Add(&S256)
require.True(t, s.IsOverHalfOrder())

rBytes := r.Bytes()
sBytes := s.Bytes()
malSigStr := make([]byte, 64)
copy(malSigStr[32-len(rBytes):32], rBytes[:])
copy(malSigStr[64-len(sBytes):64], sBytes[:])
require.False(t, pub.VerifySignature(msg, malSigStr),
"VerifyBytes incorrect with malleated & invalid S. sig=%v, key=%v",
sig,
malSigStr,
priv,
)
}
Expand Down
13 changes: 8 additions & 5 deletions crypto/keys/secp256k1/secp256k1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import (
"math/big"
"testing"

btcSecp256k1 "github.com/btcsuite/btcd/btcec"
btcSecp256k1 "github.com/btcsuite/btcd/btcec/v2"
btcecdsa "github.com/btcsuite/btcd/btcec/v2/ecdsa"
"github.com/cosmos/btcutil/base58"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -64,7 +65,7 @@ func TestSignAndValidateSecp256k1(t *testing.T) {
// ----
// Test cross packages verification
msgHash := crypto.Sha256(msg)
btcPrivKey, btcPubKey := btcSecp256k1.PrivKeyFromBytes(btcSecp256k1.S256(), privKey.Key)
btcPrivKey, btcPubKey := btcSecp256k1.PrivKeyFromBytes(privKey.Key)
// This fails: malformed signature: no header magic
// btcSig, err := secp256k1.ParseSignature(sig, secp256k1.S256())
// require.NoError(t, err)
Expand All @@ -77,9 +78,11 @@ func TestSignAndValidateSecp256k1(t *testing.T) {
ok := ecdsa.Verify(btcPubKey.ToECDSA(), msgHash, r, s)
require.True(t, ok)

sig2, err := btcPrivKey.Sign(msgHash)
sig2, err := btcecdsa.SignCompact(btcPrivKey, msgHash, false)
// Chop off compactSigRecoveryCode.
sig2 = sig2[1:]
require.NoError(t, err)
pubKey.VerifySignature(msg, sig2.Serialize())
pubKey.VerifySignature(msg, sig2)

// ----
// Mutate the signature, just one bit.
Expand All @@ -98,7 +101,7 @@ func TestSecp256k1LoadPrivkeyAndSerializeIsIdentity(t *testing.T) {

// This function creates a private and public key in the underlying libraries format.
// The private key is basically calling new(big.Int).SetBytes(pk), which removes leading zero bytes
priv, _ := btcSecp256k1.PrivKeyFromBytes(btcSecp256k1.S256(), privKeyBytes[:])
priv, _ := btcSecp256k1.PrivKeyFromBytes(privKeyBytes[:])
// this takes the bytes returned by `(big int).Bytes()`, and if the length is less than 32 bytes,
// pads the bytes from the left with zero bytes. Therefore these two functions composed
// result in the identity function on privKeyBytes, hence the following equality check
Expand Down
13 changes: 0 additions & 13 deletions crypto/keys/utils.go

This file was deleted.

17 changes: 7 additions & 10 deletions crypto/ledger/ledger_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
package ledger

import (
"errors"
"fmt"

"github.com/btcsuite/btcd/btcec"
"github.com/pkg/errors"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/ecdsa"
"github.com/cosmos/go-bip39"
"github.com/tendermint/tendermint/crypto"

Expand Down Expand Up @@ -56,7 +56,7 @@ func (mock LedgerSECP256K1Mock) GetPublicKeySECP256K1(derivationPath []uint32) (
return nil, err
}

_, pubkeyObject := btcec.PrivKeyFromBytes(btcec.S256(), derivedPriv)
_, pubkeyObject := btcec.PrivKeyFromBytes(derivedPriv)

return pubkeyObject.SerializeUncompressed(), nil
}
Expand All @@ -70,7 +70,7 @@ func (mock LedgerSECP256K1Mock) GetAddressPubKeySECP256K1(derivationPath []uint3
}

// re-serialize in the 33-byte compressed format
cmp, err := btcec.ParsePubKey(pk, btcec.S256())
cmp, err := btcec.ParsePubKey(pk)
if err != nil {
return nil, "", fmt.Errorf("error parsing public key: %v", err)
}
Expand All @@ -97,11 +97,8 @@ func (mock LedgerSECP256K1Mock) SignSECP256K1(derivationPath []uint32, message [
return nil, err
}

priv, _ := btcec.PrivKeyFromBytes(btcec.S256(), derivedPriv)
sig, err := priv.Sign(crypto.Sha256(message))
if err != nil {
return nil, err
}
priv, _ := btcec.PrivKeyFromBytes(derivedPriv)
sig := ecdsa.Sign(priv, crypto.Sha256(message))

return sig.Serialize(), nil
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/ledger/ledger_notavail.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package ledger

import (
"github.com/pkg/errors"
"errors"
)

// If ledger support (build tag) has been enabled, which implies a CGO dependency,
Expand Down

0 comments on commit acea3e4

Please sign in to comment.