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.

Sign up for GitHub

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

Jump to bottom

abci: implement process proposal to spec #9122

Merged
merged 23 commits into from
Aug 9, 2022
Merged

abci: implement process proposal to spec #9122

merged 23 commits into from
Aug 9, 2022

Conversation

cmwaters
Copy link
Contributor

Closes: #9121

Ref: #9053

JayT106 and others added 17 commits July 27, 2022 20:07
@JayT106 @sergio-mena
Cherry-picked #6567: state/types: refactor makeBlock, makeBlocks and …
51f3118 
…makeTxs (#6567)
@mconcat @sergio-mena
Rebased and git-squashed the commits in PR #7091
5cc5110 
add processproposal proto/boilerplate/logic

mockery

gofmt

fix test

gofmt

move UNKNOWN response behaviour to reject
@sergio-mena
make proto-gen
873b908
@sergio-mena
Fix ProcessProposalAsync
0a0812c
@sergio-mena
Fixed ProcessProposalSync
b8ae650
@sergio-mena
Fixed some UTs
359f6af
@sergio-mena
Added missing ProcessProposal to socket_server.go
bd8bbe8
@sergio-mena
Adapt TestProcessProposal
fbf26c9
@sergio-mena
Re-renamed TwoThirdPrevote... to Valid...
d6d61a3
@sergio-mena
Addressed William's comment on ProcessProposal error
e9d17d0
@sergio-mena
Addressed Callum's comments
fe5c4a2
@sergio-mena
Fixed base app
96ac25d
@sergio-mena
make format
14e85b2
@sergio-mena
Added missing ProcessProposal in kvstore
b9cb5e3
@sergio-mena
mockery
b2109ed
@sergio-mena
Proto-bufs for #7961
d9d6a2f
@cmwaters
follow #7961 and complete process proposal to spec
227db02
@cmwaters cmwaters changed the base branch from main to feature/abci++ppp July 28, 2022 13:20
@cmwaters cmwaters marked this pull request as ready for review August 1, 2022 08:01
@cmwaters
Copy link
Contributor Author

cmwaters commented Aug 1, 2022

Looking through the changes, I think we're light on tests in the consensus package. I would like to mock out the application and test some variations of the new calls but I want to do this in a separate PR.

sergio-mena
sergio-mena reviewed Aug 3, 2022
View reviewed changes
Copy link
Contributor

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

Awesome job!

state/execution.go Outdated Show resolved Hide resolved
state/execution_test.go Show resolved Hide resolved
consensus/mempool_test.go Outdated Show resolved Hide resolved
williambanfield
williambanfield suggested changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@williambanfield williambanfield left a comment

Choose a reason for hiding this comment

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

Should port the changes @sergio-mena suggested. Looks great beyond that.

consensus/mempool_test.go Outdated Show resolved Hide resolved
@cmwaters cmwaters requested review from a team and williambanfield August 6, 2022 17:40
@cmwaters cmwaters requested a review from sergio-mena August 6, 2022 17:40
williambanfield
williambanfield suggested changes Aug 8, 2022
View reviewed changes
Copy link
Contributor

@williambanfield williambanfield left a comment

Choose a reason for hiding this comment

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

It looks like a few changes unrelated to ABCI++ were mixed in here, some of which have already landed in main . I don't think they are necessary to do again here since this will be merged into main anyway.

A few notes otherwise, but mostly looks good.

.github/workflows/build.yml Outdated Show resolved Hide resolved
abci/types/types.go Outdated Show resolved Hide resolved
state/execution.go Outdated Show resolved Hide resolved
state/execution.go Outdated Show resolved Hide resolved
state/execution.go Outdated Show resolved Hide resolved
state/execution.go Show resolved Hide resolved
rpc/jsonrpc/server/http_server.go Outdated Show resolved Hide resolved
p2p/transport_test.go Outdated Show resolved Hide resolved
test/maverick/node/node.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
@cmwaters cmwaters force-pushed the cal/process_proposal branch from 2dd5a83 to 4be2e22 Compare August 9, 2022 12:48
@cmwaters cmwaters merged commit f861062 into feature/abci++ppp Aug 9, 2022
@cmwaters cmwaters deleted the cal/process_proposal branch August 9, 2022 13:15
samricotta pushed a commit that referenced this pull request Aug 16, 2022
@cmwaters @samricotta
abci: implement process proposal to spec (#9122)
6fe8d75
@tnasu tnasu mentioned this pull request Dec 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williambanfield williambanfield williambanfield approved these changes

@ebuchman ebuchman Awaiting requested review from ebuchman

@tychoish tychoish Awaiting requested review from tychoish

@sergio-mena sergio-mena Awaiting requested review from sergio-mena

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cmwaters @williambanfield @sergio-mena @JayT106 @mconcat