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

Merge branch feature/abci++ppp into main #9301

Merged
merged 20 commits into from
Aug 22, 2022

Conversation

sergio-mena
Copy link
Contributor

@sergio-mena sergio-mena commented Aug 22, 2022

Closes #9300

This PR is a merging branch feature/abci++ppp into main

After this PR is in, branch feature/abci++ppp will be removed (as its history is now merged into the main branch).

The contents of this PR represent the whole set of changes backported from v0.35.x/v0.36.x to deliver PrepareProposal/ProcessProposal in the next release.

(the CHANGELOG_PENDING.md update is coming in a PR to be created shortly)


PR checklist

  • Tests written/updated, or no tests needed
  • CHANGELOG_PENDING.md updated, or no changelog entry needed
  • Updated relevant documentation (docs/) and code comments, or no
    documentation updates needed

sergio-mena and others added 20 commits July 27, 2022 15:09
* [Rebased to v0.34.x] abci: PrepareProposal (#6544)

* fixed cherry-pick

* proto changes

* make proto-gen

* UT fixes

* generate Client directive

* mockery

* App fixes

* Disable 'modified tx' hack

* mockery

* Make format

* Fix lint

Co-authored-by: Marko <marbar3778@yahoo.com>
…#9157)

* Update CODEOWNERS to use teams (#9129)

* Update CODEOWNERS to use teams

Update the `CODEOWNERS` file to use the
@tendermint/tendermint-engineering and @tendermint/tendermint-research
teams as opposed to adding people one by one. This makes repository
administration somewhat easier to manage, especially when
onboarding/offboarding people.

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

* Add Ethan as superuser

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

* Prepare `main` to become new default branch (#9095)

* Update Makefile with changes from #7372

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

* Sync main GitHub config with master and update

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

* Remove unnecesary dot folders

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

* Sync dotfiles

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

* Remove unused Jepsen tests for now

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

* tools: remove k8s (#6625)

Remove mintnet as discussed on team call.

closes #1941

* Restore nightly fuzz testing of P2P addrbook and pex

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

* Fix YAML lints

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

* Fix YAML formatting nits

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

* More YAML nits

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

* github: fix linter configuration errors and occluded errors (#6400)

* Minor fixes to OpenAPI spec to sync with structs on main

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

* Remove .github/auto-comment.yml - does not appear to be used

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

* Add issue config with link to discussions

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

* Adjust issue/PR templates to suit current process

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

* Remove unused RC branch config from release workflow

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

* Fix wildcard matching in build jobs config

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

* Document markdownlint config

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

* Restore manual E2E test group config

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

* Document linter workflow with local execution instructions

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

* Document and fix minor nit in Super-Linter markdownlint config

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

* Update .github/ISSUE_TEMPLATE/bug-report.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update pull request template to add language around discussions/issues

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

* .golangci.yml: Deleted commented-out lines

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

* ci: Drop "-2" from e2e-nightly-fail workflow

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

* Address triviality concern in PR template

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

Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

Co-authored-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
…pp (#9117)

* abci: PrepareProposal-VoteExtension integration [2nd try] (#7821)

* PrepareProposal-VoteExtension integration (#6915)

* make proto-gen

* Fix protobuf crash in e2e nightly tests

* Update types/vote.go

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Addressed @creachadair's comments

Co-authored-by: mconcat <monoidconcat@gmail.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Proto changes

* make proto-gen

* Fixed UTs

* bump

* lint

* lint2

* lint3

* lint4

* lint5

* lint6

* no_lint

Co-authored-by: mconcat <monoidconcat@gmail.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
* ----start----

* [PARTIAL cherry-pick] ABCI Vote Extension 2 (#6885)

* Cherry-picked #6567: state/types: refactor makeBlock, makeBlocks and makeTxs (#6567)

* [Cherrypicked] types: remove panic from block methods (#7501)

* [cherrypicked] abci++: synchronize PrepareProposal with the newest version of the spec (#8094)

This change implements the logic for the PrepareProposal ABCI++ method call. The main logic for creating and issuing the PrepareProposal request lives in execution.go and is tested in a set of new tests in execution_test.go. This change also updates the mempool mock to use a mockery generated version and removes much of the plumbing for the no longer used ABCIResponses.

* make proto-gen

* Backported EvidenceList's method ToABCI from #7961

* make build

* Fix mockery for Mempool

* mockery

* Backported abci Application mocks from #7961

* mockery2

* Fixed new PrepareProposal test cases in state/execution_test.go

* Fixed returned errors in consensus/state.go

* lint

* Addressed @cmwaters' comment

Co-authored-by: mconcat <monoidconcat@gmail.com>
Co-authored-by: JayT106 <JayT106@users.noreply.github.com>
Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
* -----start------

* [cherrypicked] state: panic on ResponsePrepareProposal validation error (#8145)

* state: panic on ResponsePrepareProposal validation error

* lint++

Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* [cherrypicked] abci++: remove CheckTx call from PrepareProposal flow (#8176)

* [cherrypicked] abci++: correct max-size check to only operate on added and unmodified (#8242)

* [cherrypicked] Remove `ModifiedTxStatus` from the spec and the code (#8210)

* Outstanding abci-gen changes to 'pb.go' files

* Removed modified_tx_status from spec and protobufs

* Fix sed for OSX

* Regenerated abci protobufs with 'abci-proto-gen'

* Code changes. UTs e2e tests passing

* Recovered UT: TestPrepareProposalModifiedTxStatusFalse

* Adapted UT

* Fixed UT

* Revert "Fix sed for OSX"

This reverts commit e576708.

* Update internal/state/execution_test.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update internal/state/execution_test.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Addressed some comments

* Added one test that tests error at the ABCI client + Fixed some mock calls

* Addressed remaining comments

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Addressed William's latest comments

* Adressed Michael's comment

* Fixed UT

* Some md fixes

* More md fixes

* gofmt

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* make proto-gen

* Fixed testcase on PrepareProposal error

* mockery

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
…to App (#9219)

* [cherrypicked] abci++: only include meaningful header fields in data passed-through to application (#8216)

* make proto-gen

* Fix kvstore tests

* Small changes to abci protobufs taken from v0.36.x

* make proto-gen (again)

* [Partial cherrypick] Restore `Commit` to the ABCI++ spec, and other late modifications (backport #8796) (#8936)

* Restore `Commit` to the ABCI++ spec, and other late modifications (#8796)

* Added VoteExtensionsEnableHeight

* Fix reference to `modified`

* Removed old pseudo-code, now included in spec

* Removed markdown warnings in abci++_basic_concepts_002_draft.md

* Restored `Commit` in the Methods section

* Addressed remaining markdown warnings

* Revisited intro and basic concepts section

* Extra pass at all spec sections to recover Commit, and other ABCI++ spec modifications

* Fixed links

* make proto-gen

* Remove _primes_ from spec notation

* Update proto/tendermint/abci/types.proto

Co-authored-by: Callum Waters <cmwaters19@gmail.com>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: Callum Waters <cmwaters19@gmail.com>

* Addressed @cmwaters' comments

* Addressed @angbrav's and @mpoke's comments on spec

* make proto-gen

* Fix MD anchor reference

* Clarify throughout the spec when `ProcessProposal` and `VerifyVoteExtension` are called

* Update spec/abci++/abci++_app_requirements_002_draft.md

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update spec/abci++/abci++_app_requirements_002_draft.md

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update spec/abci++/abci++_app_requirements_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_basic_concepts_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_basic_concepts_002_draft.md

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update spec/abci++/abci++_basic_concepts_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_methods_002_draft.md

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Addresed comments

* Renamed 'draft' files

* Adatped links to new filenames

* Fixed links and minor cosmetic changes

* Renamed 'byzantine_validators' to 'misbehavior' in ABCI++ spec and protobufs

* make proto-gen

* Renamed 'byzantine_validators' to 'misbehavior' in the code

* Fixed link

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_basic_concepts.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Update spec/abci++/abci++_methods.md

Co-authored-by: Daniel <daniel.cason@usi.ch>

* Addressed @cason's comments

* Clarified conditions for `ProcessProposal` call at proposer

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Daniel <daniel.cason@usi.ch>
(cherry picked from commit 331860c)

* Fixed merge conflicts

Co-authored-by: Sergio Mena <sergio@informal.systems>

* make proto-gen (and again)

* make build

* fix UTs

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* [cherrypicked] version: add abci version to handshake (#5706)

- add `AbciVersion` RequestInfo

Closes: #2804

* make proto-gen

* Bump ABCI version: Prepare and Process proposal

Co-authored-by: Marko <marbar3778@yahoo.com>
* Remove set option for abci
* [cherrypicked] abci-cli: added `PrepareProposal` command to cli (#8656)

* Prepare prosal cli

* [cherrypicked + fixes] abci-cli: Add `process_proposal` command to abci-cli (#8901)

* Add `process_proposal` command to abci-cli

* Added process proposal to the 'tutorial' examples

* Added entry in CHANGELOG_PENDING.md

* Allow empty blocks in PrepareProposal, ProcessProposal, and FinalizeBlock

* Fix minimum arguments

* Add tests for empty block

* Updated abci-cli doc

Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

* Addressed @williambanfield's comment

Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Co-authored-by: Hernán Vanzetto <hernan.vanzetto@gmail.com>
)

This changes the ResponsePrepareProposal type, substituting the []TxRecord for just []bytes. This change is made in the .proto file and in all necessary additional places in the code.
@sergio-mena sergio-mena requested a review from a team August 22, 2022 15:23
Copy link
Contributor

@tychoish tychoish left a comment

Choose a reason for hiding this comment

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

this all seems good, just some minor quibbles.

I would have expected that the SetOption removal pieces would have been able to happen in an orthogonal PR, but it seems like this is fine too.

.gitignore Show resolved Hide resolved
Comment on lines +21 to +22
- [abci] \#8656 Added cli command for `PrepareProposal`. (@jmalicevic)
- [abci] \#8901 Added cli command for `ProcessProposal`. (@hvanz)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we combine these lines

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 am preparing a CHANGELOG PR spanning all PrepareProposal/ProcessProposal changes that will deal with this

Comment on lines +190 to +191
func (app *Application) ProcessProposal(
req types.RequestProcessProposal) types.ResponseProcessProposal {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think splitting this makes it harder to read, and we haven't typically enforced line lengths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this is a merge commit, I prefer not to tackle these minor issues now, but in a subsequent PR.
Recall this is already reviewed code (on the feature branch), and someone decided this was good to go.

@sergio-mena
Copy link
Contributor Author

sergio-mena commented Aug 22, 2022

I would have expected that the SetOption removal pieces would have been able to happen in an orthogonal PR, but it seems like this is fine too.

Yeah we hesitated whether to base the PR on main or on feature/abci++ppp. In the end we went for the latter, as other changes in ABCI's types.proto were going on, and wanted to avoid conflicts at the moment of merging the branch into main (i.e., now)

@sergio-mena sergio-mena self-assigned this Aug 22, 2022
@sergio-mena sergio-mena merged commit 50b5c23 into main Aug 22, 2022
@sergio-mena sergio-mena deleted the sergio/merge_abci++ppp_into_main branch August 22, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done/Merged
Development

Successfully merging this pull request may close these issues.

Merge the feature/abci++ppp branch into main once all relevant work from #9053 is done
6 participants