Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade to tm version 23.0. #1927

Merged
merged 18 commits into from
Aug 12, 2018
Merged

Upgrade to tm version 23.0. #1927

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7410bf6
Start upgrade. Currently go test ./... hangs.
ValarDragon Aug 6, 2018
c62380d
(squash this) Fix staking tests
ValarDragon Aug 7, 2018
73b4bdd
wip
ValarDragon Aug 7, 2018
57693b9
note what changes need to be made to make this work on tm v0.23.0
ValarDragon Aug 8, 2018
41d0355
Fix addr -> pubkey map
ValarDragon Aug 8, 2018
2f85fff
cleanup code
ValarDragon Aug 8, 2018
4ee35d4
Fix slashing test failures except for begin blocker
ValarDragon Aug 8, 2018
d130bbf
fix all slashing tests
ValarDragon Aug 8, 2018
a107a80
fix lcd tests
ValarDragon Aug 8, 2018
7b52940
Address PR comments
ValarDragon Aug 9, 2018
794a699
add link to changelog.
ValarDragon Aug 9, 2018
d2acf55
Merge remote-tracking branch 'origin/develop' into dev/upgrade_tm_ver…
ValarDragon Aug 9, 2018
a27a43b
(wip) start making addrToPubkey map persisted. Since amino can't hand…
ValarDragon Aug 9, 2018
f44a487
Use the correct method of storing a map
ValarDragon Aug 9, 2018
c4b6c58
(squash this) address PR comments
ValarDragon Aug 10, 2018
0e85025
Did you run 'make'?
cwgoes Aug 10, 2018
161dfaa
Merge branch 'develop' into dev/upgrade_tm_version
ebuchman Aug 12, 2018
7322cda
remove gaiadebug binary
ebuchman Aug 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 29 additions & 24 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Gopkg.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@

[[override]]
name = "github.com/tendermint/tendermint"
version = "=v0.22.6"
version = "=v0.23.0"

[[constraint]]
name = "github.com/bartekn/go-bip39"
Expand Down
7 changes: 7 additions & 0 deletions PENDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## v0.24.0 PENDING
^--- PENDING wasn't purged on sdk v0.23.0 release.

BREAKING CHANGES
* Update to tendermint v0.23.0. This involves removing crypto.Pubkey,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the PR reference here (preferably with a link por favor)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why the PR is helpful. The tendermint changelog is what is helpful, I can link that?

Copy link
Contributor

Choose a reason for hiding this comment

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

But the SDK changed and that's important to note where/how it happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this is a downside of our pending system, the commit with all the changes can't be obtained from the file. I'll link the PR then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'd just would like to see a CHANGELOG and be able to go to the issue and/or PR that made the relevant changes.

maintaining a validator address to pubkey map, and using time.Time instead of int64 for time.

## PENDING

BREAKING CHANGES
Expand Down
9 changes: 3 additions & 6 deletions baseapp/baseapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
}

// set the signed validators for addition to context in deliverTx
app.signedValidators = req.Validators
// TODO: communicate this result to the address to pubkey map in slashing
app.signedValidators = req.LastCommitInfo.GetValidators()
return
}

Expand All @@ -412,11 +413,7 @@ func (app *BaseApp) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) {
Log: result.Log,
GasWanted: result.GasWanted,
GasUsed: result.GasUsed,
Fee: cmn.KI64Pair{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we getting rid of this? Presumably Tendermint needs to know about the fee?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fees were removed.
from the changelog:
[abci] Removed Fee from ResponseDeliverTx and ResponseCheckTx

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, should we add priority then?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ValarDragon thoughts on the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason to add priority right now. We're not ready to support that at the mempool level, and we won't be for awhile. We discussed that priority is something we want to add as a way to test protobuf upgradability on a test net, not right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha.

Copy link
Contributor

@cwgoes cwgoes Aug 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

OK, I think it's not necessary to delay ABCI updates because Tendermint won't use the value yet, but nbd.

[]byte(result.FeeDenom),
result.FeeAmount,
},
Tags: result.Tags,
Tags: result.Tags,
}
}

Expand Down
3 changes: 2 additions & 1 deletion client/lcd/lcd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"regexp"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -530,7 +531,7 @@ func TestUnrevoke(t *testing.T) {
signingInfo := getSigningInfo(t, port, pkString)
tests.WaitForHeight(4, port)
require.Equal(t, true, signingInfo.IndexOffset > 0)
require.Equal(t, int64(0), signingInfo.JailedUntil)
require.Equal(t, time.Unix(0, 0).UTC(), signingInfo.JailedUntil)
require.Equal(t, true, signingInfo.SignedBlocksCounter > 0)
}

Expand Down
2 changes: 1 addition & 1 deletion client/lcd/test_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func startTM(
proxy.NewLocalClientCreator(app),
genDocProvider,
dbProvider,
nm.DefaultMetricsProvider,
nm.DefaultMetricsProvider(tmcfg.Instrumentation),
logger.With("module", "node"),
)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion client/tx/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ func SignTxRequstHandler(w http.ResponseWriter, r *http.Request) {
return
}

w.Write(sig.Bytes())
w.Write(sig)
}
6 changes: 5 additions & 1 deletion cmd/gaia/app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) ab
func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
tags := gov.EndBlocker(ctx, app.govKeeper)
validatorUpdates := stake.EndBlocker(ctx, app.stakeKeeper)

// Add these new validators to the addr -> pubkey map.
app.slashingKeeper.AddValidators(validatorUpdates)
return abci.ResponseEndBlock{
ValidatorUpdates: validatorUpdates,
Tags: tags,
Expand Down Expand Up @@ -181,6 +182,9 @@ func (app *GaiaApp) initChainer(ctx sdk.Context, req abci.RequestInitChain) abci
// return sdk.ErrGenesisParse("").TraceCause(err, "")
}

// load the address to pubkey map
slashing.InitGenesis(app.slashingKeeper, genesisState.StakeData)

gov.InitGenesis(ctx, app.govKeeper, gov.DefaultGenesisState())

return abci.ResponseInitChain{
Expand Down
Binary file modified cmd/gaia/cmd/gaiadebug/gaiadebug
View file
Open in desktop
Binary file not shown.
19 changes: 9 additions & 10 deletions crypto/encode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

tcrypto "github.com/tendermint/tendermint/crypto"
Expand All @@ -15,15 +16,17 @@ type byter interface {
Bytes() []byte
}

func checkAminoBinary(t *testing.T, src byter, dst interface{}, size int) {
func checkAminoBinary(t *testing.T, src, dst interface{}, size int) {
// Marshal to binary bytes.
bz, err := cdc.MarshalBinaryBare(src)
require.Nil(t, err, "%+v", err)
// Make sure this is compatible with current (Bytes()) encoding.
require.Equal(t, src.Bytes(), bz, "Amino binary vs Bytes() mismatch")
if byterSrc, ok := src.(byter); ok {
// Make sure this is compatible with current (Bytes()) encoding.
assert.Equal(t, byterSrc.Bytes(), bz, "Amino binary vs Bytes() mismatch")
}
// Make sure we have the expected length.
if size != -1 {
require.Equal(t, size, len(bz), "Amino binary size mismatch")
assert.Equal(t, size, len(bz), "Amino binary size mismatch")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why assert instead of require? Wouldn't we want this to fail immediately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was changing this to match whats in tendermint/crypto right now. I don't really have a preference here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's most important to be consistent -- it seems in SDK-land, the defacto call is require.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its to use require unless there is any reason not to. There is good reason to not use it here if we had more pubkey types, but I guess we can change it when that happens.

}
// Unmarshal.
err = cdc.UnmarshalBinaryBare(bz, dst)
Expand Down Expand Up @@ -55,8 +58,6 @@ func ExamplePrintRegisteredTypes() {
//| PubKeySecp256k1 | tendermint/PubKeySecp256k1 | 0xEB5AE987 | 0x21 | |
//| PrivKeyEd25519 | tendermint/PrivKeyEd25519 | 0xA3288910 | 0x40 | |
//| PrivKeySecp256k1 | tendermint/PrivKeySecp256k1 | 0xE1B0F79B | 0x20 | |
//| SignatureEd25519 | tendermint/SignatureEd25519 | 0x2031EA53 | 0x40 | |
//| SignatureSecp256k1 | tendermint/SignatureSecp256k1 | 0x7FC4A495 | variable | |
}

func TestKeyEncodings(t *testing.T) {
Expand Down Expand Up @@ -86,13 +87,11 @@ func TestKeyEncodings(t *testing.T) {
require.EqualValues(t, tc.privKey, priv3)

// Check (de/en)codings of Signatures.
var sig1, sig2, sig3 tcrypto.Signature
var sig1, sig2 []byte
sig1, err := tc.privKey.Sign([]byte("something"))
require.NoError(t, err)
checkAminoBinary(t, sig1, &sig2, -1) // Signature size changes for Secp anyways.
require.EqualValues(t, sig1, sig2)
checkAminoJSON(t, sig1, &sig3, false) // TODO also check Prefix bytes.
require.EqualValues(t, sig1, sig3)

// Check (de/en)codings of PubKeys.
pubKey := tc.privKey.PubKey()
Expand All @@ -107,7 +106,7 @@ func TestKeyEncodings(t *testing.T) {
func TestNilEncodings(t *testing.T) {

// Check nil Signature.
var a, b tcrypto.Signature
var a, b []byte
checkAminoJSON(t, &a, &b, true)
require.EqualValues(t, a, b)

Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/keybase.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (kb dbKeybase) Get(name string) (Info, error) {

// Sign signs the msg with the named key.
// It returns an error if the key doesn't exist or the decryption fails.
func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig tmcrypto.Signature, pub tmcrypto.PubKey, err error) {
func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) {
info, err := kb.Get(name)
if err != nil {
return
Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/keybase_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestSignVerify(t *testing.T) {
cases := []struct {
key crypto.PubKey
data []byte
sig crypto.Signature
sig []byte
valid bool
}{
// proper matches
Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Keybase interface {
Delete(name, passphrase string) error

// Sign some bytes, looking up the private key to use
Sign(name, passphrase string, msg []byte) (crypto.Signature, crypto.PubKey, error)
Sign(name, passphrase string, msg []byte) ([]byte, crypto.PubKey, error)

// CreateMnemonic creates a new mnemonic, and derives a hierarchical deterministic
// key from that.
Expand Down
Loading