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.

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

Update the template to SDK v0.44 #1307

Closed
fadeev opened this issue Jun 27, 2021 · 20 comments · Fixed by #1575
Closed

Update the template to SDK v0.44 #1307

fadeev opened this issue Jun 27, 2021 · 20 comments · Fixed by #1575
Assignees
Milestone

Comments

@fadeev
Copy link
Contributor

fadeev commented Jun 27, 2021

https://docs.cosmos.network/v0.43/migrations/chain-upgrade-guide-043.html

@faddat
Copy link
Contributor

faddat commented Jun 27, 2021

Nice. Didn't know that had been released, I'll look into it.

@fadeev
Copy link
Contributor Author

fadeev commented Jun 28, 2021

Only an RC has been released, but it's unlikely that there will be more breaking changes, so it's as good as released as far as Starport is concerned.

@faddat
Copy link
Contributor

faddat commented Jun 28, 2021

Alrighty. I will scaffold something, bump it, and make sure that it works correctly and if so I'll put in a PR.

@faddat
Copy link
Contributor

faddat commented Jun 28, 2021

github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer: package github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/keeper: package github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/keeper provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/types: package github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/types provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/core: package github.com/cosmos/cosmos-sdk/x/ibc/core provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/core/02-client: package github.com/cosmos/cosmos-sdk/x/ibc/core/02-client provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/core/05-port/types: package github.com/cosmos/cosmos-sdk/x/ibc/core/05-port/types provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/core/24-host: package github.com/cosmos/cosmos-sdk/x/ibc/core/24-host provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version v0.43.0-rc0
github.com/faddat/upgradever/app imports
	github.com/cosmos/cosmos-sdk/x/ibc/core/keeper: package github.com/cosmos/cosmos-sdk/x/ibc/core/keeper provided by github.com/cosmos/cosmos-sdk at latest version v0.42.6 but not at required version

just adding the sdk-go repo to the various bits since ibc moved and then we should be good to go.

@shravanshetty1
Copy link

Looking forward to this!

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

Almost finished. Just working on the antehandler and upgrade module, they've changed a bit too.

Also, feel free to follow along, as I'm starting to think this may require some restructuring. I am checking out simapp in the sdk to try and figure out what that might look like.

Issues are specifically:

  • It now expects a slightly different kvstore type.
  • Cleanup is needed on marshaling per: cosmos/cosmos-sdk@d19791b
  • There are some changes to how IBC things work

It is a larger task than I had thought, but I am enjoying it, thoroughly educational.

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

antehandlers are now like:

	anteHandler, err := ante.NewAnteHandler(
		ante.HandlerOptions{
			AccountKeeper:   app.AccountKeeper,
			BankKeeper:      app.BankKeeper,
			SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
			FeegrantKeeper:  app.FeeGrantKeeper,
			SigGasConsumer:  ante.DefaultSigVerificationGasConsumer,
		},
	)

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

@fadeev I figure we want the fee grant module and authz, correct?

@fadeev
Copy link
Contributor Author

fadeev commented Jun 29, 2021

@faddat since we currently don't have a way to properly import modules, I'd say it's fine to have those in the standard template.

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

They're in.

IBCKeeper is fixed and.... lord knows I'll need to fix another few things.

https://github.com/cosmos/ibc-go/blob/2e95805e2d474b85c837ec433e64947782039047/docs/migrations/ibc-migration-043.md

useful doc!

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

cgo-gcc-prolog:203:11: warning: 'SecTrustedApplicationCreateFromPath' is deprecated: first deprecated in macOS 10.15 - No longer supported [-Wdeprecated-declarations]
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Security.framework/Headers/SecTrustedApplication.h:59:10: note: 'SecTrustedApplicationCreateFromPath' has been explicitly marked deprecated here
# github.com/faddat/supersptest/app
app/app.go:34:2: imported and not used: "github.com/cosmos/cosmos-sdk/x/authz"
app/app.go:427:5: app.Configurator undefined (type *App has no field or method Configurator)
: exit status 2

only small errors remain. Excitement!

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

It'll break wasm a bit I think, this is from root.go:

// initAppConfig helps to override default appConfig template and configs.
// return "", nil if no custom configuration is required for the application.
func initAppConfig() (string, interface{}) {
	// The following code snippet is just for reference.

	// WASMConfig defines configuration for the wasm module.
	type WASMConfig struct {
		// This is the maximum sdk gas (wasm and storage) that we allow for any x/wasm "smart" queries
		QueryGasLimit uint64 `mapstructure:"query_gas_limit"`

		// Address defines the gRPC-web server to listen on
		LruSize uint64 `mapstructure:"lru_size"`
	}

	type CustomAppConfig struct {
		serverconfig.Config

		WASM WASMConfig `mapstructure:"wasm"`
	}

	// Optionally allow the chain developer to overwrite the SDK's default
	// server config.
	srvCfg := serverconfig.DefaultConfig()
	// The SDK's default minimum gas price is set to "" (empty value) inside
	// app.toml. If left empty by validators, the node will halt on startup.
	// However, the chain developer can set a default app.toml value for their
	// validators here.
	//
	// In summary:
	// - if you leave srvCfg.MinGasPrices = "", all validators MUST tweak their
	//   own app.toml config,
	// - if you set srvCfg.MinGasPrices non-empty, validators CAN tweak their
	//   own app.toml to override, or use this default value.
	//
	// In simapp, we set the min gas prices to 0.
	srvCfg.MinGasPrices = "0stake"

	customAppConfig := CustomAppConfig{
		Config: *srvCfg,
		WASM: WASMConfig{
			LruSize:       1,
			QueryGasLimit: 300000,
		},
	}

	customAppTemplate := serverconfig.DefaultConfigTemplate + `
[wasm]
# This is the maximum sdk gas (wasm and storage) that we allow for any x/wasm "smart" queries
query_gas_limit = 300000
# This is the number of wasm vm instances we keep cached in memory for speed-up
# Warning: this is currently unstable and may lead to crashes, best to keep for 0 unless testing locally
lru_size = 0`

	return customAppTemplate, customAppConfig
}

So what I am going to do is have it return "" as instructed. I guess when importing wasm, all of the above will be needed.

When someone imports wasm, we will need to add all of that. Since we import wasm at Juno.... I'll do it as part of

CosmosContracts/juno#23

and

CosmosContracts/juno#24

Basically what I'm saying is that I'll break, then fix wasm support.

@faddat
Copy link
Contributor

faddat commented Jun 29, 2021

@fadeev

So, the template upgrade is finished, but we now need to upgrade Starport. The format that keys are outputted in has changed, resulting in:

{"name":"alice","type":"local","address":"cosmos15dw8wh4h9zgw3jf0qwyjvkux5zm65dgngzfzs3","pubkey":"{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"Ao3d7z0zWOu5MgtTFFZbqhid6AuqHjmNDaK67+ZCoz4/\"}","mnemonic":"pull divorce debate height wagon attitude speed decide high essay lawsuit theme stereo learn sausage skull language spice feel jelly sugar salon call message"}
EOF

I believe it's that "type" field, I think it wasn't there before.

@faddat
Copy link
Contributor

faddat commented Aug 4, 2021

2 things make this much harder:

  1. spm is separate from the template (I can't find any reason why that is)
  2. go.sum in the template

@fadeev
Copy link
Contributor Author

fadeev commented Aug 4, 2021

spm contains things that we abstracted away from the template, because they should always be part of the chain: appd init, appd start, etc. Users shouldn't be messing with commands that are required for the chain to function. It's in a separate repo, because if it was part of Starport, the template would have to import all of Starport, which is huge.

spm is also a very bad name and we should rename it 😁

It also allows us to add features to users' chains without asking them to modify the template. See #1417

@faddat
Copy link
Contributor

faddat commented Aug 4, 2021

ahhhhhhhhhh.

OK, good point about the import.

However, I'm now looking at forking spm for various things. I don't think that's how it is intended, but it is how it's working out.

@faddat
Copy link
Contributor

faddat commented Aug 4, 2021

spm: starport module right?

@faddat
Copy link
Contributor

faddat commented Aug 4, 2021

Back to the topic here-- there is something missing from my work, and I unfortunately do not know what it is.

I thought that the crash was only related to text formatting on the secp keys, but I was wrong.

I scaffolded a chain with the code in the linked PR, and I did the old fashioned

init
add account
gentx
collect gentxs
start

and unfortunately I still got a pretty epic crash.

So I have decided that I am going to try and upgrade dig (https://github.com/faddat/dig) itself, then have a second look at the template. I felt like I was probably missing something pretty easy. I might pull spm into dig (vendor it into the repo) for ease of use while doing this.

@fadeev
Copy link
Contributor Author

fadeev commented Aug 4, 2021

Well, if there is something in your fork that is useful for others, we'll consider adding these features to spm proper. Normally, a user shouldn't need to mess with node's cmd.

@fadeev fadeev added this to the v0.18 milestone Aug 23, 2021
@fadeev fadeev changed the title Update the template to SDK v0.43 Update the template to SDK v0.44 Sep 6, 2021
@Pantani
Copy link
Collaborator

Pantani commented Sep 6, 2021

also, we should update the bandchain-packet to v0.0.2

@ilgooz ilgooz linked a pull request Sep 15, 2021 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants