Modify ExportCmd to export genesis with bech32_prefix #402

anhductn2001 · 2024-04-26T06:51:06Z

PR Standards

Since Exported type of Cosmos-SDK default return format is GenesisDoc which does not include the bech32_prefix field, we need to modify it to allow bech32_prefix to be exported every time we export genesis

Opening a pull request should be able to meet the following requirements

Closes #XXX

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.

For Author:

Targeted PR against correct branch
included the correct type prefix in the PR title
Linked to Github issue with discussion and accepted design
Targets only one github issue
Wrote unit and integration tests
All CI checks have passed
Added relevant godoc comments

For Reviewer:

confirmed the correct type prefix in the PR title
Reviewers assigned
confirmed all author checklist items have been addressed

After reviewer approval:

In case targets main branch, PR should be squashed and merged.
In case PR targets a release branch, PR should be rebased.

Co-authored-by: Trinity <[email protected]>

Co-authored-by: Omri <[email protected]> Co-authored-by: danwt <[email protected]>

…399)

danwt · 2024-04-26T08:37:43Z

Is there an issue?

danwt · 2024-04-26T08:37:50Z

linter fails btw

anhductn2001 · 2024-04-26T08:39:23Z

yea, it relate to: dymensionxyz/rollapp-evm#178

danwt

I'm kind of missing the TLDR here
what is the problem being solved?

server/commands.go

danwt · 2024-04-26T08:39:07Z

server/commands.go

@@ -19,7 +20,7 @@ import (
 )

 // add Rollapp commands


wrong docstr

It was a previous comment, but I'll add some comments to it

danwt · 2024-04-26T08:39:45Z

server/export.go

+	"os"
+	"strconv"
+
+	rdk_genutiltypes "github.com/dymensionxyz/dymension-rdk/x/genutil/types"


danwt · 2024-04-26T08:40:51Z

server/commands.go

+func AddRollappCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator types.AppCreator, appExport rdk_types.AppExporter, addStartFlags types.ModuleInitFlags) {
 	dymintCmd := &cobra.Command{


does this have usages?

Yes, see it: https://github.com/dymensionxyz/rollapp-evm/pull/188/files#diff-3ef291e710a554004d50d61607fe864a27e6b4350f3cb764f2cff8b442c2c213L215

danwt · 2024-04-26T08:42:36Z

server/export.go

+				if _, err := fmt.Fprintln(os.Stderr, "WARNING: App exporter not defined. Returning genesis file."); err != nil {
+					return err
+				}
+
+				genesis, err := os.ReadFile(config.GenesisFile())
+				if err != nil {
+					return err
+				}
+
+				fmt.Println(string(genesis))
+				return nil
+			}
+
+			genDoc, err := rdk_genutiltypes.GenesisDocFromFile(config.GenesisFile())
+			if err != nil {
+				fmt.Println("Failed to read genesis doc from file", err)


why sometimes stderr, sometimes stdout?

it's just a function taken from cosmos-sdk with a little change

danwt · 2024-04-26T08:42:50Z

server/export.go

+
+			exported, err := appExporter(serverCtx.Logger, db, traceWriter, height, forZeroHeight, jailAllowedAddrs, serverCtx.Viper)
+			if err != nil {
+				return fmt.Errorf("error exporting state: %v", err)


Suggested change

return fmt.Errorf("error exporting state: %v", err)

return fmt.Errorf("exporting state: %v", err)

I think we shouldn't change it

danwt · 2024-04-26T08:43:03Z

server/export.go

+
+			exported, err := appExporter(serverCtx.Logger, db, traceWriter, height, forZeroHeight, jailAllowedAddrs, serverCtx.Viper)
+			if err != nil {
+				return fmt.Errorf("error exporting state: %v", err)


Suggested change

return fmt.Errorf("error exporting state: %v", err)

return fmt.Errorf("exporting state: %w", err)

I think we shouldn't change it

danwt · 2024-04-26T08:43:21Z

server/export.go

+			consensus_param := doc["consensus_params"].(map[string]interface{})
+			block := consensus_param["block"].(map[string]interface{})
+			time_iota_ms, _ := strconv.ParseInt(block["time_iota_ms"].(string), 10, 64)


no snake

Need to check OK for type assertion?

danwt · 2024-04-26T08:43:52Z

server/types/app.go

+	// the expected types and could result in type assertion errors. It is recommend
+	// to either use the cast package or perform manual conversion for safety.
+	AppOptions interface {
+		Get(string) interface{}


Suggested change

Get(string) interface{}

Get(string) any

danwt · 2024-04-26T08:53:29Z

x/genutil/types/genesis_state.go

+// for the application.
+//
+// NOTE: The pubkey input is this machines pubkey.
+func GenesisStateFromGenFile(genFile string) (genesisState map[string]json.RawMessage, genDoc map[string]interface{}, err error) {


if you do concrete type assertions without checking ok on the genesisState, can we have a concrete type instead?

anhductn2001 · 2024-04-26T10:19:09Z

I have updated description of this PR.

anhductn2001 and others added 6 commits April 23, 2024 18:14

feat: Add collect-gentxs command to pickup custom gendoc (#388)

f6997e0

Co-authored-by: Trinity <[email protected]>

feat: permissions management across modules (#392)

8d63aac

feat!: add governors module instead of x/staking module (#394)

a544721

Co-authored-by: Omri <[email protected]> Co-authored-by: danwt <[email protected]>

build(deps): updated dymint to v1.1.0-rc01 (#398)

a39708d

Revert "feat!: add governors module instead of x/staking module" (#…

19ba05a

…399)

modify ExportCmd

7ad2446

anhductn2001 requested a review from a team as a code owner April 26, 2024 06:51

anhductn2001 added 3 commits April 26, 2024 14:07

modify ExportAppStateAndValidators

faf405a

get bech32 from genesis

a4db59f

interface {} is string, not int64

9b181fe

lint

0f2af2c

danwt reviewed Apr 26, 2024

View reviewed changes

anhductn2001 added 2 commits April 26, 2024 17:12

lint

368ce99

add comment

74ceaae

danwt self-requested a review April 26, 2024 16:25

omritoptix force-pushed the main branch from 98d128a to 842a5b9 Compare May 5, 2024 13:24

anhductn2001 closed this May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify ExportCmd to export genesis with bech32_prefix #402

Modify ExportCmd to export genesis with bech32_prefix #402

anhductn2001 commented Apr 26, 2024 •

edited

Loading

danwt commented Apr 26, 2024

danwt commented Apr 26, 2024

anhductn2001 commented Apr 26, 2024

danwt left a comment

danwt Apr 26, 2024

anhductn2001 Apr 26, 2024

danwt Apr 26, 2024

danwt Apr 26, 2024

anhductn2001 Apr 26, 2024

danwt Apr 26, 2024

anhductn2001 Apr 26, 2024

danwt Apr 26, 2024

anhductn2001 Apr 26, 2024

danwt Apr 26, 2024

anhductn2001 Apr 26, 2024

danwt Apr 26, 2024

danwt Apr 26, 2024

danwt Apr 26, 2024

anhductn2001 commented Apr 26, 2024

		func AddRollappCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator types.AppCreator, appExport rdk_types.AppExporter, addStartFlags types.ModuleInitFlags) {
		dymintCmd := &cobra.Command{

	return fmt.Errorf("error exporting state: %v", err)
	return fmt.Errorf("exporting state: %v", err)

	return fmt.Errorf("error exporting state: %v", err)
	return fmt.Errorf("exporting state: %w", err)

Modify ExportCmd to export genesis with bech32_prefix #402

Modify ExportCmd to export genesis with bech32_prefix #402

Conversation

anhductn2001 commented Apr 26, 2024 • edited Loading

PR Standards

Opening a pull request should be able to meet the following requirements

danwt commented Apr 26, 2024

danwt commented Apr 26, 2024

anhductn2001 commented Apr 26, 2024

danwt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anhductn2001 commented Apr 26, 2024

anhductn2001 commented Apr 26, 2024 •

edited

Loading