avalanchego icon indicating copy to clipboard operation
avalanchego copied to clipboard

Pull Genesis parsing out of state

Open abi87 opened this issue 1 year ago • 5 comments

Why this should be merged

Building setup for unit test in platformVm is cumbersome. Once of the reasons is that we use genesisBytes to initialize the platformVM state. This forces us to build genesis and serialize it in UTs. This PR drop the bytes and pass a genesisState data structure, thus simplifying the UTs context. This PR kills 130 LOC without giving up any functionality. Thanks @joshua-kim and @marun for the discussions that brought me to these changes.

How this works

Let state.State accept a genesis state instead of its bytes, moving the unmarshalling to the client

How this was tested

CI

abi87 avatar Nov 09 '23 18:11 abi87

Why does the configuration for testing have to mirror non-test scenarios?

It's not like the temporary networks we use for e2e testing mirror the deployment semantics for fuji or mainnet.

marun avatar Dec 18 '23 19:12 marun

Why does the configuration for testing have to mirror non-test scenarios?

This PR modifies non-test files (specifically, adjusting the state struct to take in a genesis struct instead of bytes)

dhrubabasu avatar Dec 18 '23 19:12 dhrubabasu

I believe our current model for VMs is they are initialized with genesis bytes: https://docs.avax.network/build/vm/create/golang-vm-complex#virtual-machine-methods

This feels like a step in the wrong direction. I think to simplify testing, we could instead have a defaultGenesisBytes as a UT constant.

@dhrubabasu I am not changing the VM interface. Rather I am moving the byte parsing out of the state package. I think right now the state package is under tested. One of the reasons is that is pretty curmbersome to create ad-hoc genesis content for tests. I want to write test genesis with no validators for instance, which are only useful in tests and while I can to it now, I have to go through the whole byte formattin shenanigans. Having a simpler struct where I directly specify the genesis content would make for much smaller tests. Also defaultGenesisBytes as a UT constant would not offer enough flexibility for testing IMO

abi87 avatar Dec 19 '23 09:12 abi87

I think we can create a helper function that returns the genesisBytes. We shouldn't need to modify the state struct to create a test genesis with no validators. For example, this code works and uses the static service we use in defaultGenesis now. I feel abstracting this into a genesistest pkg helper would consolidate a lot of the code without modifying non-test code.

func main() {
	buildGenesisArgs := api.BuildGenesisArgs{
		Encoding:      formatting.Hex,
		NetworkID:     json.Uint32(constants.UnitTestID),
		AvaxAssetID:   ids.GenerateTestID(),
		UTXOs:         []api.UTXO{}, // No UTXOs
		Validators:    []api.GenesisPermissionlessValidator{}, // No Validators
		Chains:        nil,
		Time:          json.Uint64(time.Date(1997, 1, 1, 0, 0, 0, 0, time.UTC).Unix()),
		InitialSupply: json.Uint64(360 * units.MegaAvax),
	}

	buildGenesisResponse := api.BuildGenesisReply{}
	platformvmSS := api.StaticService{}
	err := platformvmSS.BuildGenesis(nil, &buildGenesisArgs, &buildGenesisResponse)
	if err != nil {
		panic(err)
	}

	genesisBytes, err := formatting.Decode(buildGenesisResponse.Encoding, buildGenesisResponse.Bytes)
	if err != nil {
		panic(err)
	}

	fmt.Println(genesisBytes)
}
[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 50 201 169 0 4 254 250 23 183 36 0 0 0 0]

dhrubabasu avatar Dec 19 '23 15:12 dhrubabasu

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will cause this PR to ignore lifecycle events.

github-actions[bot] avatar Mar 03 '24 00:03 github-actions[bot]