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

[Feature]: Allow creation of AppGenesis without a file lookup #17570

Closed
chatton opened this issue Aug 29, 2023 · 2 comments · Fixed by #17571
Closed

[Feature]: Allow creation of AppGenesis without a file lookup #17570

chatton opened this issue Aug 29, 2023 · 2 comments · Fixed by #17571

Comments

@chatton
Copy link
Contributor

chatton commented Aug 29, 2023

Summary

Provide an alternative to AppGenesisFromFile which does not depend on a file lookup to create an instance of AppGenesis.

Problem Definition

In the ibc-go e2e tests, we use interchain test which provides a hook to modify the genesis bytes. In order to provide modifications, we need to serialize the bytes, make changes, and return the new bytes.

The current implementation of AppGenesisFromFile does a direct os.ReadFile and so is not quite flexible enough to meet our use case.

Proposed Feature

I propose the method signature is changed (or a new fn is added is the original behaviour is still required!)

I think using an io.Reader is a nice ux for this.

// AppGenesisFromReader reads the AppGenesis from the reader.
func AppGenesisFromReader(reader io.Reader) (*AppGenesis, error) {
	jsonBlob, err := io.ReadAll(reader)
	if err != nil {
		return nil, err
	}

	var appGenesis AppGenesis
	if err := json.Unmarshal(jsonBlob, &appGenesis); err != nil {
		// fallback to CometBFT genesis
		var ctmGenesis cmttypes.GenesisDoc
		if err2 := cmtjson.Unmarshal(jsonBlob, &ctmGenesis); err2 != nil {
			return nil, fmt.Errorf("error unmarshalling AppGenesis: %w\n failed fallback to CometBFT GenDoc: %w", err, err2)
		}

		appGenesis = AppGenesis{
			AppName: version.AppName,
			// AppVersion is not filled as we do not know it from a CometBFT genesis
			GenesisTime:   ctmGenesis.GenesisTime,
			ChainID:       ctmGenesis.ChainID,
			InitialHeight: ctmGenesis.InitialHeight,
			AppHash:       ctmGenesis.AppHash,
			AppState:      ctmGenesis.AppState,
			Consensus: &ConsensusGenesis{
				Validators: ctmGenesis.Validators,
				Params:     ctmGenesis.ConsensusParams,
			},
		}
	}

	return &appGenesis, nil
}

// AppGenesisFromFile reads the AppGenesis from the provided file.
func AppGenesisFromFile(genFile string) (*AppGenesis, error) {
	file, err := os.Open(genFile)
	if err != nil {
		return nil, err
	}
	return AppGenesisFromReader(bufio.NewReader(file))
}
@tac0turtle
Copy link
Member

want to open a pr and we can include it in 0.50 since its not api breaking?

@chatton
Copy link
Contributor Author

chatton commented Aug 29, 2023

can do! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants