feat(e2e): allow customizing genesis parameters on the manifest by cason · Pull Request #3969 · cometbft/cometbft

cason · 2024-09-02T15:54:08Z

Closes #3961.

Similarly to #3967, introduces a new genesis field for manifest files. The format is key = value and multiple entries can be provided. For example,

genesis = [ 
    "consensus_params.evidence.max_bytes = 1000", 
    "chain_id = pr3969" 
]

After the genesis document is produced by the setup implementation of the runner, the provided genesis customized configurations are applied to the generated genesis file. The solution also uses viper, as it allows setting only some specific keys, leaving the pre-produced content untouched.

Note: this PR is based atop #3964 for simplicity, but it does not need to.

PR checklist

Tests written/updated
Changelog entry added in .changelog (we use unclog to manage our changelog)
Updated relevant documentation (docs/ or spec/) and code comments

melekes

👍

cason · 2024-09-03T08:39:18Z

There is a lint error I don't have idea what it means:

 golangci-lint-full.......................................................Failed
- hook id: golangci-lint-full
- exit code: 1

WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar. 
test/e2e/runner/setup.go:18:2: import 'github.com/mitchellh/mapstructure' is not allowed from list 'main' (depguard)
	"github.com/mitchellh/mapstructure"
	^

andynog

Same comment made in #3967

We need to validate the key with a valid field in genesis otherwise we might get undesired behaviors

(e2e) > ./build/runner -f networks/single.toml setup
I[2024-09-03|14:34:21.615] setup                                        msg="Generating testnet files in `networks/single`"
D[2024-09-03|14:34:21.616] Applying Genesis config                      consensus_params.evidence.max_bytes=1000
D[2024-09-03|14:34:21.616] Applying Genesis config                      chain_id=pr3969

here chain_id was set in the genesis.json file, but mispelling the field name to chainid

(e2e) > ./build/runner -f networks/single.toml setup
I[2024-09-03|14:34:32.827] setup                                        msg="Generating testnet files in `networks/single`"
D[2024-09-03|14:34:32.827] Applying Genesis config                      consensus_params.evidence.max_bytes=1000
D[2024-09-03|14:34:32.827] Applying Genesis config                      chainid=pr3969

validation accepted invalid field name, so of course it was not set

{
  "genesis_time": "2024-09-03T14:34:32.827736Z",
  "chain_id": "single",

andynog

I will just unblock this, my previous comment is still valid but the implementation to solve the validation problem is not easy. Might need a future PR if we can get it to work.

cason · 2024-09-04T13:50:19Z

test/e2e/pkg/testnet.go

 		tokens := strings.Split(entry, " = ")
 		if len(tokens) != 2 {
 			return fmt.Errorf("invalid config entry: \"%s\", "+
+


Suggested change

…eys (#4016) Addresses comments #3969 (review) and #3967 (review) on previous PRs. The solution is in fact simple, as it is possible to tell viper to return an error if one of the configured entries (key/value pairs) were not applied to the destination structure, because they don't exist or are invalid. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

Closes #3961. Similarly to #3967, introduces a new `genesis` field for manifest files. The format is `key = value` and multiple entries can be provided. For example, ``` genesis = [ "consensus_params.evidence.max_bytes = 1000", "chain_id = pr3969" ] ``` After the genesis document is produced by the `setup` implementation of the runner, the provided genesis customized configurations are applied to the generated genesis file. The solution also uses `viper`, as it allows setting only some specific keys, leaving the pre-produced content untouched. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev> (cherry picked from commit 6c5cd32) # Conflicts: # test/e2e/pkg/manifest.go # test/e2e/pkg/testnet.go # test/e2e/runner/setup.go

…eys (#4016) Addresses comments #3969 (review) and #3967 (review) on previous PRs. The solution is in fact simple, as it is possible to tell viper to return an error if one of the configured entries (key/value pairs) were not applied to the destination structure, because they don't exist or are invalid. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev> (cherry picked from commit bff1667) # Conflicts: # test/e2e/pkg/testnet.go # test/e2e/runner/setup.go

Closes #3961. Similarly to #3967, introduces a new `genesis` field for manifest files. The format is `key = value` and multiple entries can be provided. For example, ``` genesis = [ "consensus_params.evidence.max_bytes = 1000", "chain_id = pr3969" ] ``` After the genesis document is produced by the `setup` implementation of the runner, the provided genesis customized configurations are applied to the generated genesis file. The solution also uses `viper`, as it allows setting only some specific keys, leaving the pre-produced content untouched. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

…eys (#4016) Addresses comments #3969 (review) and #3967 (review) on previous PRs. The solution is in fact simple, as it is possible to tell viper to return an error if one of the configured entries (key/value pairs) were not applied to the destination structure, because they don't exist or are invalid. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

…port #3969) (#4312) Closes #3961. Similarly to #3967, introduces a new `genesis` field for manifest files. The format is `key = value` and multiple entries can be provided. For example, ``` genesis = [ "consensus_params.evidence.max_bytes = 1000", "chain_id = pr3969" ] ``` After the genesis document is produced by the `setup` implementation of the runner, the provided genesis customized configurations are applied to the generated genesis file. The solution also uses `viper`, as it allows setting only some specific keys, leaving the pre-produced content untouched. Note: this PR is based atop #3964 for simplicity, but it does not need to. --- This is an manual backport of pull request #3969. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments

…eys (backport #4016) (#4313) Addresses comments #3969 (review) and #3967 (review) on previous PRs. The solution is in fact simple, as it is possible to tell viper to return an error if one of the configured entries (key/value pairs) were not applied to the destination structure, because they don't exist or are invalid. --- This is an manual backport of pull request #4016. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

Daniel Cason added 6 commits September 2, 2024 13:09

e2e: rename Manifest fields colliding with Testnet

4991c80

e2e: Tesnet includes and extends Manifest

92f9ec1

e2e: Node includes and extends ManifestNode

0a596f2

e2e: fix Database Node setting

778fbc6

e2e: added genesis parameter to Manifest

972bcf8

e2e: runner sets Genesis parameters from Testnet

486dcbe

cason requested a review from a team as a code owner September 2, 2024 15:54

cason requested a review from a team September 2, 2024 15:54

Merge branch 'main' into cason/e2e-refactor

2072bad

cason self-assigned this Sep 2, 2024

cason added the e2e Related to our end-to-end tests label Sep 2, 2024

mergify bot and others added 2 commits September 2, 2024 15:57

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

449cd3a

e2e: added some comments

fde7484

cason linked an issue Sep 2, 2024 that may be closed by this pull request

e2e: allow customizing genesis parameters on the manifest #3961

Closed

cason changed the title ~~e2e: allow customizing genesis parameters on the manifest~~ feat(e2e): allow customizing genesis parameters on the manifest Sep 2, 2024

melekes approved these changes Sep 3, 2024

View reviewed changes

mergify bot and others added 7 commits September 3, 2024 08:47

Merge branch 'main' into cason/e2e-refactor

6e8de84

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

af6c175

Merge branch 'main' into cason/e2e-refactor

71b18ae

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

2d32712

Merge branch 'main' into cason/e2e-refactor

a476a03

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

900e3f4

pleasing the linter

12d545e

andynog suggested changes Sep 3, 2024

View reviewed changes

Merge branch 'main' into cason/e2e-refactor

c0da912

andynog approved these changes Sep 3, 2024

View reviewed changes

mergify bot added 3 commits September 3, 2024 21:00

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

8755a73

Merge branch 'main' into cason/e2e-refactor

eb40ed4

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

dba7279

Merge branch 'cason/e2e-refactor' into cason/3961-e2e-genesis

c735eb5

Base automatically changed from cason/e2e-refactor to main September 4, 2024 13:17

Merge branch 'main' into cason/3961-e2e-genesis

4851f15

cason enabled auto-merge September 4, 2024 13:20

cason added this pull request to the merge queue Sep 4, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 4, 2024

Merge branch 'main' into cason/3961-e2e-genesis

82e933d

cason commented Sep 4, 2024

View reviewed changes

Finxing merging conflicts

9f0d10f

cason enabled auto-merge September 4, 2024 13:51

cason added this pull request to the merge queue Sep 4, 2024

Merged via the queue into main with commit 6c5cd32 Sep 4, 2024

cason deleted the cason/3961-e2e-genesis branch September 4, 2024 14:03

cason mentioned this pull request Sep 6, 2024

fix(e2e): viper errors with invalid config and genesis manifest keys #4016

Merged

3 tasks

cason added the backport-to-v1.x label Oct 20, 2024

This was referenced Oct 20, 2024

feat(e2e): allow customizing genesis parameters on the manifest (backport #3969) #4310

Closed

fix(e2e): viper errors with invalid config and genesis manifest keys (backport #4016) #4311

Closed

cason added backport-to-v1.x and removed backport-to-v1.x labels Oct 20, 2024

cason mentioned this pull request Oct 20, 2024

feat(e2e): allow customizing genesis parameters on the manifest (backport #3969) #4312

Merged

6 tasks

cason mentioned this pull request Oct 20, 2024

fix(e2e): viper errors with invalid config and genesis manifest keys (backport #4016) #4313

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(e2e): allow customizing genesis parameters on the manifest#3969

feat(e2e): allow customizing genesis parameters on the manifest#3969
cason merged 25 commits intomainfrom
cason/3961-e2e-genesis

cason commented Sep 2, 2024 •

edited

Loading

Uh oh!

melekes left a comment

Uh oh!

cason commented Sep 3, 2024

Uh oh!

andynog left a comment

Uh oh!

andynog left a comment

Uh oh!

cason Sep 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cason commented Sep 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

melekes left a comment

Choose a reason for hiding this comment

Uh oh!

cason commented Sep 3, 2024

Uh oh!

andynog left a comment

Choose a reason for hiding this comment

Uh oh!

andynog left a comment

Choose a reason for hiding this comment

Uh oh!

cason Sep 4, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cason commented Sep 2, 2024 •

edited

Loading