Skip to content

Conversation

otrack
Copy link
Contributor

@otrack otrack commented Jul 17, 2023

This PR addresses issue #1059.

It is built atop #1043.

Summary:
This PR adds a support for fast prototyping to the E2E test suite. Please read the REAME.md located here to start using this code.

Progress:
To date, the following goals are in full (or partially) done:


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

thanethomson and others added 24 commits July 13, 2023 11:05
Signed-off-by: Thane Thomson <connect@thanethomson.com>
* Revert "config: add bootstrap peers (#9680)"

This reverts commit f12588a.

* docs/p2p: bootstrap_peers config flag removed
* Revert "Remove unused code (cometbft#286)"

This reverts commit a2d9915.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* node: Remove access to consensus state

Consensus state should only ever be accessible via the consensus
reactor.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix mistake in changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
* Remove UPnP functionality

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update documentation and specs to reflect UPnP removal

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
* ADR 107: Rename proto versions to pre-v1 betas

* ADR 107: fix hyperlinks to ADR 103

* ADR 107: authorship of the revision

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* ADR 107: change status to Accepted

---------

Co-authored-by: Thane Thomson <connect@thanethomson.com>
* Add first draft

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Expand comment on actor receive method

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix grammar

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Emphasize/clarify conclusions

Signed-off-by: Thane Thomson <connect@thanethomson.com>

---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
…tbft#1131)

Bumps [github.com/bufbuild/buf](https://github.com/bufbuild/buf) from 1.23.1 to 1.24.0.
- [Release notes](https://github.com/bufbuild/buf/releases)
- [Changelog](https://github.com/bufbuild/buf/blob/main/CHANGELOG.md)
- [Commits](bufbuild/buf@v1.23.1...v1.24.0)

---
updated-dependencies:
- dependency-name: github.com/bufbuild/buf
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ometbft#1132)

Bumps [github.com/vektra/mockery/v2](https://github.com/vektra/mockery) from 2.31.1 to 2.32.0.
- [Release notes](https://github.com/vektra/mockery/releases)
- [Changelog](https://github.com/vektra/mockery/blob/master/docs/changelog.md)
- [Commits](vektra/mockery@v2.31.1...v2.32.0)

---
updated-dependencies:
- dependency-name: github.com/vektra/mockery/v2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…etbft#1133)

Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2.9.0 to 2.9.1.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@v2.9.0...v2.9.1)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…metbft#1134)

Bumps [bufbuild/buf-setup-action](https://github.com/bufbuild/buf-setup-action) from 1.23.1 to 1.24.0.
- [Release notes](https://github.com/bufbuild/buf-setup-action/releases)
- [Commits](bufbuild/buf-setup-action@v1.23.1...v1.24.0)

---
updated-dependencies:
- dependency-name: bufbuild/buf-setup-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Add new small docker image (new make target)
- Add one example of custom mempool reactor (gossip w. tunnable propagation rate)
- Provide (initial) support for e2e tests in fast-prototyping mode
- Add new (customizable) benchmark
- Provide scripts to collect and process metrics
@otrack otrack requested a review from a team as a code owner July 17, 2023 16:07
@otrack otrack requested a review from a team July 17, 2023 16:07
@otrack otrack added wip Work in progress P:bandwidth-optimization Priority: Optimize bandwidth usage labels Jul 17, 2023
@otrack otrack marked this pull request as draft July 17, 2023 16:09
Copy link
Contributor

@lasarojc lasarojc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While there are improvements to be done, we could do them in the cometbft repo.

// Inject custom reactors (see node/main.go#startNode for a list of possibilities)
ExperimentalCustomReactors map[string]string `toml:"experimental_custom_reactors"`

// Experimental gossip "propagation rate" and "send once" features
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if "experimental" should be part of the variable name or just in the comments of both the code and template config file.
On the good side of doing so, it will be impossible for someone to use the flag without knowing it is experimental. On the bad side, once it is graduated from experimental, there is a lot more updating to do, including the config files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this helps to understand that the feature is not available in the base reactor. The change is not that difficult when merging.

Another approach would be to add a set of experimental parameters. My feeling is that it is a bit heavy weighted.

@cason was proposing also that instead of having experimental reactors under fast-prototyping/reactors, to have them in different branches. Such a solution is also fully do-able.

@@ -0,0 +1,20 @@
package gossip
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest renaming this package/reactor to simpleGossip as gossip is very generic and will likely be component of every other reactor variant.

@lasarojc
Copy link
Contributor

Awesome work.
Althought there are a few things to improve, this is already very useful and IMHO we should bring this into a feature branch in the CometBFT repo and finish the work there.

@otrack
Copy link
Contributor Author

otrack commented Jul 31, 2023

Awesome work. Althought there are a few things to improve, this is already very useful and IMHO we should bring this into a feature branch in the CometBFT repo and finish the work there.

Thanks a lot :-)

I plan to commit the PR today, and will let you create a feature branch based on it.

@otrack otrack marked this pull request as ready for review July 31, 2023 16:57
@otrack otrack requested a review from a team as a code owner July 31, 2023 16:57
- load: change info reporting when failing to sync with a node
@otrack otrack merged commit 62e3137 into cometbft:pierre/fast-prototyping-1059 Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P:bandwidth-optimization Priority: Optimize bandwidth usage wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants