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

chore: bump sdk to v1.20.0 #2860

Merged
merged 13 commits into from
Nov 22, 2023
+36 −57
Merged

chore: bump sdk to v1.20.0 #2860

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
20009fd
chore: bump sdk to v1.20.0
cmwaters Nov 20, 2023
fc378bd
fix compile errors
cmwaters Nov 20, 2023
9536657
try to get the v2 upgrade to work
cmwaters Nov 21, 2023
a6b82fc
bump to rc1
cmwaters Nov 21, 2023
856c42f
take 3
cmwaters Nov 21, 2023
69f9d6a
fix the test to make it work
cmwaters Nov 21, 2023
9dec0bb
gofumpt
cmwaters Nov 21, 2023
16e071a
Merge branch 'main' into cal/bump-sdk
cmwaters Nov 21, 2023
fa90c36
use block header version
cmwaters Nov 21, 2023
e60aed5
fix integration tests
cmwaters Nov 21, 2023
71dd65a
use v0.12.4 instead
cmwaters Nov 22, 2023
6cb16da
Merge branch 'main' into cal/bump-sdk
cmwaters Nov 22, 2023
1dae88c
use the actual v1.20.0 version
cmwaters Nov 22, 2023
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
5 changes: 1 addition & 4 deletions app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo
// NOTE: this is a specific feature for upgrading to v2 as v3 and onward is expected
// to be coordinated through the signalling protocol
if app.UpgradeKeeper.ShouldUpgrade(req.Height) {
app.SetProtocolVersion(v2.Version)
app.SetAppVersion(ctx, v2.Version)
staheri14 marked this conversation as resolved.
Show resolved Hide resolved
}
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
return res
}
Expand All @@ -587,9 +587,6 @@ func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.Res
if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
panic(err)
}
if req.ConsensusParams != nil && req.ConsensusParams.Version != nil {
app.SetProtocolVersion(req.ConsensusParams.Version.AppVersion)
}
Comment on lines -590 to -592
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this no longer necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's done in the SDK instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://github.com/celestiaorg/cosmos-sdk/blob/18e768c968e05660e4ad0a7f4cc3ea8a33d5c993/baseapp/abci.go#L64

Copy link
Collaborator

Choose a reason for hiding this comment

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

Assuming it can be done in either location (app or SDK), why do it in the SDK where it has the risk of getting overwritten when we pull upstream changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this behaviour is logically the responsibility of baseapp not app

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then can it be upstreamed to cosmos/cosmos-sdk so it doesn't accidentally get reverted in celestiaorg/cosmos-sdk when we pull upstream changes?

return app.mm.InitGenesis(ctx, app.appCodec, genesisState)
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
2 changes: 1 addition & 1 deletion app/prepare_proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr

// build the square from the set of valid and prioritised transactions.
// The txs returned are the ones used in the square and block
dataSquare, txs, err := square.Build(txs, app.GetBaseApp().AppVersion(), app.GovSquareSizeUpperBound(sdkCtx))
dataSquare, txs, err := square.Build(txs, app.GetBaseApp().AppVersion(sdkCtx), app.GovSquareSizeUpperBound(sdkCtx))
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion app/process_proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
}

// Construct the data square from the block's transactions
dataSquare, err := square.Construct(req.BlockData.Txs, app.GetBaseApp().AppVersion(), app.GovSquareSizeUpperBound(sdkCtx))
dataSquare, err := square.Construct(req.BlockData.Txs, app.GetBaseApp().AppVersion(sdkCtx), app.GovSquareSizeUpperBound(sdkCtx))
if err != nil {
logInvalidPropBlockError(app.Logger(), req.Header, "failure to compute data square from transactions:", err)
return reject()
Expand Down
4 changes: 2 additions & 2 deletions app/square_size.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ func (app *App) GovSquareSizeUpperBound(ctx sdk.Context) int {

gmax := int(app.BlobKeeper.GovMaxSquareSize(ctx))
// perform a secondary check on the max square size.
if gmax > appconsts.SquareSizeUpperBound(app.AppVersion()) {
gmax = appconsts.SquareSizeUpperBound(app.AppVersion())
if gmax > appconsts.SquareSizeUpperBound(app.AppVersion(ctx)) {
gmax = appconsts.SquareSizeUpperBound(app.AppVersion(ctx))
}

return gmax
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ require (
)

replace (
github.com/cosmos/cosmos-sdk => github.com/celestiaorg/cosmos-sdk v1.18.3-sdk-v0.46.14
github.com/cosmos/cosmos-sdk => github.com/celestiaorg/cosmos-sdk v1.20.0-sdk-v0.46.16-rc0
Copy link
Collaborator

Choose a reason for hiding this comment

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

[not blocking] why not use an official release of celestiaorg/cosmos-sdk? If rc-0 is just for testing, can we test pre merge instead of post-merge?

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 plan to cut v1.20.0 once I've tested that the app version works as expected. I opened this PR so docker could build the image I need to run the e2e tests (apparently opening them in draft mode doesn't work)

github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1
github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7
github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.29.0-tm-v0.34.29
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ github.com/celestiaorg/blobstream-contracts/v3 v3.1.0 h1:h1Y4V3EMQ2mFmNtWt2sIhZI
github.com/celestiaorg/blobstream-contracts/v3 v3.1.0/go.mod h1:x4DKyfKOSv1ZJM9NwV+Pw01kH2CD7N5zTFclXIVJ6GQ=
github.com/celestiaorg/celestia-core v1.29.0-tm-v0.34.29 h1:Fd7ymPUzExPGNl2gZw4i5S74arMw+iDHLE78M/cCxl4=
github.com/celestiaorg/celestia-core v1.29.0-tm-v0.34.29/go.mod h1:xrICN0PBhp3AdTaZ8q4wS5Jvi32V02HNjaC2EsWiEKk=
github.com/celestiaorg/cosmos-sdk v1.18.3-sdk-v0.46.14 h1:+Te28r5Zp4Vp69f82kcON9/BIF8f1BNXb0go2+achuc=
github.com/celestiaorg/cosmos-sdk v1.18.3-sdk-v0.46.14/go.mod h1:Og5KKgoBEiQlI6u56lDLG191pfknIdXctFn3COWLQP8=
github.com/celestiaorg/cosmos-sdk v1.20.0-sdk-v0.46.16-rc0 h1:KyuLAqq4qd0MkoA+wN8tr/3msUWgt4Kc/rxCZ5Zfm6U=
github.com/celestiaorg/cosmos-sdk v1.20.0-sdk-v0.46.16-rc0/go.mod h1:Tvsc3YnqvflXTYC8xIy/Q07Es95xZ1pZC/imoKogtbg=
github.com/celestiaorg/knuu v0.10.0 h1:uYO6hVsoCAJ/Q4eV0Pn8CLbbupGAjD56xQc4y2t4lf0=
github.com/celestiaorg/knuu v0.10.0/go.mod h1:2NjDX29x09hRpaaWaKvi7pw9VjI/SHo+/4HldyEW50g=
github.com/celestiaorg/merkletree v0.0.0-20210714075610-a84dc3ddbbe4 h1:CJdIpo8n5MFP2MwK0gSRcOVlDlFdQJO1p+FqdxYzmvc=
Expand Down
2 changes: 1 addition & 1 deletion test/util/malicious/out_of_order_prepare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (a *App) OutOfOrderPrepareProposal(req abci.RequestPrepareProposal) abci.Re

// build the square from the set of valid and prioritised transactions.
// The txs returned are the ones used in the square and block
dataSquare, txs, err := Build(txs, a.GetBaseApp().AppVersion(), a.GovSquareSizeUpperBound(sdkCtx), OutOfOrderExport)
dataSquare, txs, err := Build(txs, a.GetBaseApp().AppVersion(sdkCtx), a.GovSquareSizeUpperBound(sdkCtx), OutOfOrderExport)
if err != nil {
panic(err)
}
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
9 changes: 5 additions & 4 deletions x/upgrade/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/test/util"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
Expand All @@ -21,11 +22,11 @@ func TestUpgradeAppVersion(t *testing.T) {

testApp.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 2}})
// app version should not have changed yet
require.EqualValues(t, 1, testApp.AppVersion())
require.EqualValues(t, 1, testApp.AppVersion(sdk.Context{}))
respEndBlock := testApp.EndBlock(abci.RequestEndBlock{Height: 2})
// now the app version changes
require.EqualValues(t, 2, respEndBlock.ConsensusParamUpdates.Version.AppVersion)
require.EqualValues(t, 2, testApp.AppVersion())
require.EqualValues(t, 2, testApp.AppVersion(sdk.Context{}))
}

func setupTestApp(t *testing.T, upgradeHeight int64) (*app.App, keyring.Keyring) {
Expand All @@ -40,7 +41,7 @@ func setupTestApp(t *testing.T, upgradeHeight int64) (*app.App, keyring.Keyring)

stateBytes, err := json.MarshalIndent(genesisState, "", " ")
require.NoError(t, err)
require.EqualValues(t, 0, testApp.GetBaseApp().AppVersion())
require.EqualValues(t, 0, testApp.GetBaseApp().AppVersion(sdk.Context{}))

cp := app.DefaultConsensusParams()
abciParams := &abci.ConsensusParams{
Expand All @@ -64,7 +65,7 @@ func setupTestApp(t *testing.T, upgradeHeight int64) (*app.App, keyring.Keyring)
)

// assert that the chain starts with version provided in genesis
require.EqualValues(t, app.DefaultConsensusParams().Version.AppVersion, testApp.GetBaseApp().AppVersion())
require.EqualValues(t, app.DefaultConsensusParams().Version.AppVersion, testApp.GetBaseApp().AppVersion(sdk.Context{}))

_ = testApp.Commit()
return testApp, kr
Expand Down
Loading