-
Notifications
You must be signed in to change notification settings - Fork 96
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
Conversation
why is ci not running any longer? |
…-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
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
|
||
// ParseConnectionIDFromEvents parses events emitted from a MsgConnectionOpenInit or | ||
// MsgConnectionOpenTry and returns the connection identifier. | ||
func ParseConnectionIDFromEvents(events []abci.Event) (string, error) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can check here -> https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#xfeegrant
store := runtime.NewKVStoreService(keepers.GetKVStoreKey()[types.StoreKey]).OpenKVStore(ctx) | ||
|
||
// list code in testnet | ||
listCode := []string{ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
cmd/picad/cmd/root.go
Outdated
"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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
custom/staking/keeper/keeper.go
Outdated
matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) | ||
matureUnbonds, err := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) | ||
if err != nil { | ||
panic(err) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, my mistake
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
No description provided.