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

fead: update to sdk50 #510

Merged
merged 118 commits into from
May 24, 2024
Merged

Conversation

hoank101
Copy link
Collaborator

No description provided.

@blasrodri
Copy link

why is ci not running any longer?

@hoank101 hoank101 changed the base branch from feat/sdk50 to develop2 May 16, 2024 06:00
hoank101 and others added 7 commits May 16, 2024 13:22
…-cosmos into feat/sdk50

# Conflicts:
#	app/app.go
#	app/upgrades/v6_5_5/constants.go
#	app/upgrades/v6_6_4/upgrade.go
#	app/upgrades/v6_6_4/upgrades_test.go
#	bech32-migration/gov/gov.go
#	bech32-migration/ica/ica.go
#	custom/ibc-transfer/keeper/keeper.go
#	go.mod
#	go.sum
#	scripts/test-upgrade.sh
#	scripts/upgrade/init-deps.sh
#	scripts/upgrade/upgrade.sh
#	x/transfermiddleware/ibc_middleware.go
* feat: fix scripts run node

* feat: save state

* fix: push wasm code gov

* feat: add store wasm code

* test: use correct version go.mod

* fix: store wasm code 50

* update relayer hyperspace raw scripts

* feat: use notional fork to handle decode grandpa url

* change the name of creatae clients file

* chore: change delay period to 10

* fix type url grandpa not registered

* chore: add hyperspace and cwgranpda wasm file

* feat: update make file

* docs: add how to reproduce the error

* fix miss chain-id

* feat: add scripts to config and start relayer

* fix: correcting go sum

* deps: update go version

* test: setup relayer with sdk 50

* setup gas auto

* update script test-upgrade

---------

Co-authored-by: kienn6034 <[email protected]>
* feat: add polkadot js

* feat: add polkadot js to test

* fix: register subspace ibc transfer

---------

Co-authored-by: kienn6034 <[email protected]>
* feat: bump ibc 08 wasm

* deps: using correct ibc08 wasm version

* feat: scripts to migrate contract

* chore: delete test.json
@hoank101 hoank101 changed the title update sdk50 fead: update to sdk50 May 22, 2024
custom/staking/keeper/keeper.go Fixed Show fixed Hide fixed
custom/staking/keeper/keeper.go Fixed Show fixed Hide fixed
Comment on lines +769 to +776
for _, m := range app.mm.Modules {
if moduleWithName, ok := m.(module.HasName); ok {
moduleName := moduleWithName.Name()
if appModule, ok := moduleWithName.(appmodule.AppModule); ok {
modules[moduleName] = appModule
}
}
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
app/export.go Fixed Show fixed Hide fixed
app/export.go Fixed Show fixed Hide fixed

// ParseConnectionIDFromEvents parses events emitted from a MsgConnectionOpenInit or
// MsgConnectionOpenTry and returns the connection identifier.
func ParseConnectionIDFromEvents(events []abci.Event) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ParseClientIDFromEvents and ParseConnectionIDFromEvents similar function.
Reasonable to combine them together to not duplicate the code.

}

// Used for various debug statements above when needed... do not remove
// func showEvent(evt abci.Event) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented code could be removed.

Height: height,
}
}
// func (chain *TestChain) CreateWasmClientHeader(chainID string, blockHeight int64, trustedHeight clienttypes.Height, timestamp time.Time, tmValSet, _, tmTrustedVals *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) *wasmtypes.Header {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that commented code necessary?


"github.com/cosmos/cosmos-sdk/x/feegrant"
"cosmossdk.io/x/feegrant"
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it official?
where i could find the link to this site on official cosmos github to be sure that it is not malicious 3rd party sourcecode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

store := runtime.NewKVStoreService(keepers.GetKVStoreKey()[types.StoreKey]).OpenKVStore(ctx)

// list code in testnet
listCode := []string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we are going first to devnet and then to testnet.
Am I right that the listCode for testnet will be different for devnet.
So this chain upgrade will panic on devnet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

list code in testnet,
will update listCode when update to devnet, mainnet

checksum := types.Checksums{Checksums: listCheckSum}
bz, err := codec.Marshal(&checksum)
if err != nil {
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be useful to add info to panic on what exact item from listcode it failed to decode or set into the storage here above.

"github.com/notional-labs/composable/v6/app"
// "github.com/notional-labs/composable/v6/app/params"
// this line is used by starport scaffolding # stargate/root/import
)

const (
// if set, than uses specific key for governance instead of default (default is production; this override for local devtest)
flagDevnetGov = "devnet-gov"
Copy link
Collaborator

Choose a reason for hiding this comment

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

keep in mind to remove when merge into mainnet branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

// when transfer via sdk transfer from A (module) -> B (contract)
coinToSendToB = sdk.NewCoin(sdk.DefaultBondDenom, transferAmount)
timeoutHeight = clienttypes.NewHeight(1, 110)
originAmt, err = sdk.NewIntFromString("10000000001100000000000")
originAmt, err = sdkmath.NewIntFromString("100000004100001000000")
Copy link
Collaborator

Choose a reason for hiding this comment

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

why different value? just interesting to know.

Copy link
Collaborator Author

@hoank101 hoank101 May 24, 2024

Choose a reason for hiding this comment

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

just change some balance and inc number validator in test-hepper

@@ -0,0 +1,124 @@
package keeper
Copy link
Collaborator

Choose a reason for hiding this comment

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

why we did not have this file before for custom bank?
And why we need it now?
what changed?
is it because of addressCodec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

with sdk50 we need custom send and MultiSend, it not custom will send fail

matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time)
matureUnbonds, err := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time)
if err != nil {
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this panic indeed could halt the chain.
Is the any way to handle error other way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

already refactor


func (am AppModule) EndBlock(ctx sdk.Context, _abc abcitype.RequestEndBlock) []abcitype.ValidatorUpdate {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No. you can't do this way.
you remove EndBlock for custom staking.
It means that you paused the epoch staking feature that was introduced by this EndBlock logic in custom staking.
I can't approve this changes. bz it will shut down very important feature for eth bridge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thank you, my mistake

Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please return this fn the same way it was.
this fn contains a custom end blocker epoch staking to change voting power not each block if someone unstake or stake but custom logic will change voting power depends on config(30 mins.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

And then let's merge, once you return this fn back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)

return k.BlockValidatorUpdates(ctx, ctx.BlockHeight())
return k.BlockValidatorUpdates(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
@hoank101 hoank101 merged commit 69a6072 into ComposableFi:develop2 May 24, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants