-
Notifications
You must be signed in to change notification settings - Fork 259
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
abci: add preprocess block #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a first pass. Awesome work!
I think we should populate the lazyledger specific fields directly instead of introducing a MetaData field in Block.Data.
|
||
messages := types.MessagesFromProto(pbmessages) | ||
|
||
return state.MakeBlock(height, processedTxs, evidence, nil, messages, commit, proposerAddr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master tendermint/spec#110 +/- ##
==========================================
+ Coverage 60.81% 62.51% +1.70%
==========================================
Files 261 145 -116
Lines 24216 12092 -12124
==========================================
- Hits 14726 7559 -7167
+ Misses 7954 3718 -4236
+ Partials 1536 815 -721
|
…ledger-core into marko/preprocess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @marbar3778! Thanks a lot.
LGTM
- name: Set GOBIN | ||
run: | | ||
echo "::add-path::$(go env GOPATH)/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this too! 🙏
DeliverTx(RequestDeliverTx) ResponseDeliverTx // Deliver a tx for full processing | ||
EndBlock(RequestEndBlock) ResponseEndBlock // Signals the end of a block, returns changes to the validator set | ||
Commit() ResponseCommit // Commit the state and return the application Merkle root hash | ||
PreprocessTxs(RequestPreprocessTxs) ResponsePreprocessTxs // State machine preprocessing of txs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move this in between InitChain
and BeginBlock
. But as this is only the interface it does not block merging this PR.
message ResponsePreprocessTxs { | ||
repeated bytes txs = 1; | ||
tendermint.types.Messages messages = 2; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a follow-ip issue, this will also return/include the intermediate state roots. Then, in another PR, this will enable immediate execution (ref #3 and https://github.com/tendermint/spec/issues/208#issuecomment-721158558)
MessageEmpty = Message{} | ||
MessagesEmpty = Messages{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these vars won't be used outside of this package? If so, they should be unexported.
block, _ := state.MakeBlock(height, makeTxs(height), nil, | ||
nil, types.Messages{}, lastCommit, state.Validators.GetProposer().Address) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional nit: I think, here and in other places, where the line becomes too large, we should rather use one line per argument. I think that improves the readability compared to breaking the line somewhere in the middle.
Didn't this use to be convention in the tendermint code-base anyway?
The failing test, seems to be a non-deterministic flaky test unrelated to the changes here: tendermint/tendermint#1040 |
* version docs * stash commits * fix url * remove master from sidebar * fix build errors * move to consensus connection * add process * add proof to block * add block metadata in beginBlcok * fix testing * regenerate mocks * add in needed proto changes * regenerate mocks * fix makeblock calls * Apply suggestions from code review * fix rpc tests * fix linting and ci errors * fix consensus tests * add preprocess to e2e * add preprocess to counter app * replace meta_data with messages * fix linting * Update state/execution.go Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * fix comment * fix e2e tests Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* version docs * stash commits * fix url * remove master from sidebar * fix build errors * move to consensus connection * add process * add proof to block * add block metadata in beginBlcok * fix testing * regenerate mocks * add in needed proto changes * regenerate mocks * fix makeblock calls * Apply suggestions from code review * fix rpc tests * fix linting and ci errors * fix consensus tests * add preprocess to e2e * add preprocess to counter app * replace meta_data with messages * fix linting * Update state/execution.go Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * fix comment * fix e2e tests Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* version docs * stash commits * fix url * remove master from sidebar * fix build errors * move to consensus connection * add process * add proof to block * add block metadata in beginBlcok * fix testing * regenerate mocks * add in needed proto changes * regenerate mocks * fix makeblock calls * Apply suggestions from code review * fix rpc tests * fix linting and ci errors * fix consensus tests * add preprocess to e2e * add preprocess to counter app * replace meta_data with messages * fix linting * Update state/execution.go Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * fix comment * fix e2e tests Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> fix test so that it doesn't modify block data after generating the hash fix missed make block api change run make mockery add some docs fix hardcoded test fix linter fix hard coded test
* version docs * stash commits * fix url * remove master from sidebar * fix build errors * move to consensus connection * add process * add proof to block * add block metadata in beginBlcok * fix testing * regenerate mocks * add in needed proto changes * regenerate mocks * fix makeblock calls * Apply suggestions from code review * fix rpc tests * fix linting and ci errors * fix consensus tests * add preprocess to e2e * add preprocess to counter app * replace meta_data with messages * fix linting * Update state/execution.go Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> * fix comment * fix e2e tests Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com> fix test so that it doesn't modify block data after generating the hash fix missed make block api change run make mockery add some docs fix hardcoded test fix linter fix hard coded test
* [backport] Renaming in docker-related files (#123) * [cherry-picked] Renaming in docker-related files (#110) * Performed renaming in docker-related files * Fix e2e launch * Rename TENDERMINT_BUILD_OPTIONS to COMETBFT_BUILD_OPTIONS all over * Revert `cometbft-proof` to `tendermint-proof` * Replaced TENDERMINT_VERSION --> COMETBFT_VERSION all over the codebase * [backport] Rename inside `test/e2e` (#139) (#144) * Renamed all help text for cometbft CLI * Update networks/local/localnode/Dockerfile Co-authored-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Thane Thomson <connect@thanethomson.com>
* [cherry-picked] Renaming in docker-related files (celestiaorg#110) * Performed renaming in docker-related files * Fix e2e launch * one more * revert rename of tendermint-docker.yml * unrevert renaming tendermint-docker.yml * Rename TENDERMINT_BUILD_OPTIONS to COMETBFT_BUILD_OPTIONS all over * Linter error * Revert `cometbft-proof` to `tendermint-proof` * Replaced TENDERMINT_VERSION --> COMETBFT_VERSION all over the codebase
Description
This pr adds preprocess block to abci
Closes: #XXX
ref: tendermint/spec#59