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

feat!: sync with mainline ABCI #1003

Merged
merged 13 commits into from
Jul 20, 2023
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

celestia-core is a fork of [cometbft/cometbft](https://github.com/cometbft/cometbft), an implementation of the Tendermint protocol, with the following changes:

1. Early adoption of the ABCI++ methods: `PrepareProposal` and `ProcessProposal` because they haven't yet landed in a CometBFT release.
1. Modifications to how `DataHash` in the block header is determined. In CometBFT, `DataHash` is based on the transactions included in a block. In Celestia, block data (including transactions) are erasure coded into a data square to enable data availability sampling. In order for the header to contain a commitment to this data square, `DataHash` was modified to be the Merkle root of the row and column roots of the erasure coded data square. See [ADR 008](https://github.com/celestiaorg/celestia-core/blob/v0.34.x-celestia/docs/celestia-architecture/adr-008-updating-to-tendermint-v0.35.x.md?plain=1#L20) for the motivation or [celestia-app/pkg/da/data_availability_header.go](https://github.com/celestiaorg/celestia-app/blob/2f89956b22c4c3cfdec19b3b8601095af6f69804/pkg/da/data_availability_header.go) for the implementation. Note on the implementation: celestia-app computes the hash in prepare_proposal and returns it to CometBFT via [`blockData.Hash`](https://github.com/celestiaorg/celestia-app/blob/5bbdac2d3f46662a34b2111602b8f964d6e6fba5/app/prepare_proposal.go#L78) so a modification to celestia-core isn't strictly necessary but [comments](https://github.com/celestiaorg/celestia-core/blob/2ec23f804691afc196d0104616e6c880d4c1ca41/types/block.go#L1041-L1042) were added.
1. Modifications to how `DataHash` in the block header is determined. In CometBFT, `DataHash` is based on the transactions included in a block. In Celestia, block data (including transactions) are erasure coded into a data square to enable data availability sampling. In order for the header to contain a commitment to this data square, `DataHash` was modified to be the Merkle root of the row and column roots of the erasure coded data square. See [ADR 008](https://github.com/celestiaorg/celestia-core/blob/v0.34.x-celestia/docs/celestia-architecture/adr-008-updating-to-tendermint-v0.35.x.md?plain=1#L20) for the motivation or [celestia-app/pkg/da/data_availability_header.go](https://github.com/celestiaorg/celestia-app/blob/2f89956b22c4c3cfdec19b3b8601095af6f69804/pkg/da/data_availability_header.go) for the implementation. This is computed by the application in `PrepareProposal` and returned to `CometBFT` as the second to last transcation. The last transaction is the big endian encoded uint64 of the square size. This is included in the modified `Data` struct that is gossiped to peers. Similarly `CometBFT` passes the `DataHash` as the second to last tx and the `SquareSize` as the final transaction in `ProcessProposal`.
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
2. A content-addressable transaction pool (CAT) was implemented using the `Mempool` interface to reduce the duplication and thus bandwidth of peers sending transactions to one another. The specification can be found [here](./mempool/cat/spec.md).
cmwaters marked this conversation as resolved.
Show resolved Hide resolved


See [./docs/celestia-architecture](./docs/celestia-architecture/) for architecture decision records (ADRs) on Celestia modifications.
Expand Down
4 changes: 2 additions & 2 deletions abci/example/kvstore/persistent_kvstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (app *PersistentKVStoreApplication) BeginBlock(req types.RequestBeginBlock)

// Punish validators who committed equivocation.
for _, ev := range req.ByzantineValidators {
if ev.Type == types.EvidenceType_DUPLICATE_VOTE {
if ev.Type == types.MisbehaviorType_DUPLICATE_VOTE {
addr := string(ev.Validator.Address)
if pubKey, ok := app.valAddrToPubKeyMap[addr]; ok {
app.updateValidator(types.ValidatorUpdate{
Expand Down Expand Up @@ -180,7 +180,7 @@ func (app *PersistentKVStoreApplication) PrepareProposal(

func (app *PersistentKVStoreApplication) ProcessProposal(
req types.RequestProcessProposal) types.ResponseProcessProposal {
return types.ResponseProcessProposal{Result: types.ResponseProcessProposal_ACCEPT}
return types.ResponseProcessProposal{Status: types.ResponseProcessProposal_ACCEPT}
}

//---------------------------------------------
Expand Down
16 changes: 13 additions & 3 deletions abci/types/application.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package types

import (
context "golang.org/x/net/context"
"context"
"encoding/binary"

"github.com/tendermint/tendermint/crypto/tmhash"
)

// Application is an interface that enables any finite, deterministic state machine
Expand Down Expand Up @@ -98,11 +101,18 @@ func (BaseApplication) ApplySnapshotChunk(req RequestApplySnapshotChunk) Respons
}

func (BaseApplication) PrepareProposal(req RequestPrepareProposal) ResponsePrepareProposal {
return ResponsePrepareProposal{BlockData: req.BlockData}
squareSize := len(req.Txs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] why is the squareSize set to the number of transactions? I think of these as two distinct values. For example, a block with 100 send transactions could have a square size of 4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is a good point. So I've made the decision here that I want celestia-core to be specific to celestia-app. Concretely, we extend the rules around PrepareProposal and ProcessProposal that there must always be two transactions returned and the last and second to last are the squareSize and dataHash respectively. It's obviously somewhat hacky but means we keep within the boundaries of ABCI and are able to use the SDK without requiring a fork.

If the rule isn't followed, consensus panics. I could have chosen to simply ignore the rules when an application sends back 0 txs (thus supporting all the test applications in Tendermint) but I felt better to actually enforce the rule thus I had to modify all the test applications including the BaseApplication.

The square size and hash here are placeholders. I could use the value 0 (which is invalid in the case of Celestia) instead. This should not be used outside of testing

Copy link
Collaborator

Choose a reason for hiding this comment

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

[optional] Thanks for clarifying that they are placeholders. If they are expected to be overwritten by celestia-app, then proposal to do something like:

func (BaseApplication) PrepareProposal(req RequestPrepareProposal) ResponsePrepareProposal {
	placeholderDataHash := tmhash.Sum(nil)
	placeholderSquareSizeBytes := make([]byte, 8)
	binary.BigEndian.PutUint64(placeholderSquareSizeBytes, uint64(1))

	// placeholderDataHash and placeholderSquareSizeBytes are expected to be
	// overwritten by celestia-app.
	req.Txs = append(req.Txs, placeholderDataHash, placeholderSquareSizeBytes)
	return ResponsePrepareProposal{Txs: req.Txs}
}

if squareSize == 0 {
squareSize = 1
}
squareSizeBytes := make([]byte, 8)
binary.BigEndian.PutUint64(squareSizeBytes, uint64(squareSize))
req.Txs = append(req.Txs, tmhash.Sum(nil), squareSizeBytes)
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
return ResponsePrepareProposal{Txs: req.Txs}
}

func (BaseApplication) ProcessProposal(req RequestProcessProposal) ResponseProcessProposal {
return ResponseProcessProposal{Result: ResponseProcessProposal_ACCEPT}
return ResponseProcessProposal{Status: ResponseProcessProposal_ACCEPT}
}

//-------------------------------------------------------
Expand Down
8 changes: 4 additions & 4 deletions abci/types/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ func (r ResponseQuery) IsErr() bool {

// IsUnknown returns true if Code is Unknown
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
func (r ResponseProcessProposal) IsUnknown() bool {
return r.Result == ResponseProcessProposal_UNKNOWN
return r.Status == ResponseProcessProposal_UNKNOWN
}

// IsOK returns true if Code is OK
cmwaters marked this conversation as resolved.
Show resolved Hide resolved
func (r ResponseProcessProposal) IsOK() bool {
return r.Result == ResponseProcessProposal_ACCEPT
func (r ResponseProcessProposal) IsAccepted() bool {
return r.Status == ResponseProcessProposal_ACCEPT
}

// IsRejected returns true if this ResponseProcessProposal was rejected
func (r ResponseProcessProposal) IsRejected() bool {
return r.Result == ResponseProcessProposal_REJECT
return r.Status == ResponseProcessProposal_REJECT
}

//---------------------------------------------------------------------------
Expand Down
Loading