From a8b0e5c37bdb071dee368f81d3f337395f82e6f8 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 3 Mar 2022 17:39:35 -0600 Subject: [PATCH 01/32] init commit for ADR 001 ABCI++ --- docs/architecture/ADR-001-ABCI++.md | 416 ++++++++++++++++++++++++++++ docs/architecture/adr-template.md | 72 +++++ 2 files changed, 488 insertions(+) create mode 100644 docs/architecture/ADR-001-ABCI++.md create mode 100644 docs/architecture/adr-template.md diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md new file mode 100644 index 0000000000..1d91b65430 --- /dev/null +++ b/docs/architecture/ADR-001-ABCI++.md @@ -0,0 +1,416 @@ +# ADR 001: ABCI++ Adoption + +## Changelog + +- 03/03/2022: Initial Commit + +## Context + +Amongst other things, ABCI++ enables arbitrary logic to be performed by the application to create and verify proposal blocks. + +We need this functionality in order for block producers to + +- Pick a square size / Fill the data square efficiently +- Malleate user transactions +- Separate rollup block data from the stateful portion of the transaction +- Use the appropriate signature for the selected block size +- Follow the non-interactive default rules (not done here) +- Create celestia specific data hash by erasuring the block data + +We also need this functionality for validators to verify that + +- Messages that are paid for by users are actually included in the block +- The block producer doesn’t also include messages that are not paid for +- That the data hash represents the properly erasured block data for the selected block size +- Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) + +Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the work for us, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by creating a simplified version of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. + +## Alternative Approaches + +While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++. + +Alternatives for Message Inclusion +https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches + +Alternatives for Picking a square size +https://github.com/celestiaorg/celestia-core/issues/454 + +## Decision + +Proposed and initial implementation is complete. + +## Detailed Design + +### [#631](https://github.com/celestiaorg/celestia-core/pull/631) Simplified version of ABCI++ (`celestia-core`) + +In order to be a viable medium term solution, we want to make as few changes as possible to both the cosmos-sdk and the app. This is done by only incorporating the two necessary new ABCI++ methods that pulled from upstream. Only pulling these two methods from upstream allows for these new methods to be easily be implemented in the cosmos-sdk/app, not unlike we previously added `PreprocessTxs`. `state/execution.go` + +```go +// Application is an interface that enables any finite, deterministic state machine +// to be driven by a blockchain-based replication engine via the ABCI. +type Application interface { + ... + PrepareProposal(RequestPrepareProposal) ResponsePrepareProposal + ProcessProposal(RequestProcessProposal) ResponseProcessProposal + ... +} +``` + +It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modifed to pass the entire block data. +```protobuf +message RequestPrepareProposal { + // block_data is an array of transactions that will be included in a block, + // sent to the app for possible modifications. + // applications can not exceed the size of the data passed to it. + tendermint.types.Data block_data = 1; + // If an application decides to populate block_data with extra information, they can not exceed this value. + int64 block_data_size = 2; +} + +message RequestProcessProposal { + tendermint.types.Header header = 1 [(gogoproto.nullable) = false]; + tendermint.types.Data block_data = 2; +} + +... + +// Data contains the set of transactions, evidence, intermediate state roots, +// and messages to be included in the block +message Data { + // Txs that will be applied by state @ block.Height+1. + // NOTE: not all txs here are valid. We're just agreeing on the order first. + // This means that block.AppHash does not include these txs. + repeated bytes txs = 1; + + IntermediateStateRoots intermediate_state_roots = 2 [(gogoproto.nullable) = false]; + EvidenceList evidence = 3 [(gogoproto.nullable) = false]; + Messages messages = 4 [(gogoproto.nullable) = false]; + uint64 original_square_size = 5; + bytes hash = 6; // <-- The hash has been added so that the erasure step can be performed in the app +} +``` + +Here is where the new ABCI++ method is called to perform arbitrary logic in the app before creating the proposal block. `consensus/state.go` + +```go +// CreateProposalBlock calls state.MakeBlock with evidence from the evpool +// and txs from the mempool. +func (blockExec *BlockExecutor) CreateProposalBlock( + height int64, + state State, commit *types.Commit, + proposerAddr []byte, +) (*types.Block, *types.PartSet) { + ... + txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas) + l := len(txs) + bzs := make([][]byte, l) + for i := 0; i < l; i++ { + bzs[i] = txs[i] + } + + preparedProposal, err := blockExec.proxyApp.PrepareProposalSync( + abci.RequestPrepareProposal{ + BlockData: &tmproto.Data{Txs: txs.ToSliceOfBytes(), Evidence: *pevdData}, + BlockDataSize: maxDataBytes}, + ) + ... + return state.MakeBlock( + ... + ) +} +``` + +Here is where arbitrary logic is performed by the app before voting on a proposed block during consensus. + +```go +func (cs *State) defaultDoPrevote(height int64, round int32) { + ... + stateMachineValidBlock, err := cs.blockExec.ProcessProposal(cs.ProposalBlock) + if err != nil { + cs.Logger.Error("state machine returned an error when trying to process proposal block", "err", err) + } + + // Vote nil if application invalidated the block + if !stateMachineValidBlock { + // Consensus says we must vote nil + logger.Error("prevote step: consensus deems this block to be mustVoteNil", "err", err) + cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) + return + } + + // Prevote cs.ProposalBlock + // NOTE: the proposal signature is validated when it is received, + // and the proposal block parts are validated as they are received (against the merkle hash in the proposal) + logger.Debug("prevote step: ProposalBlock is valid") + cs.signAddVote(tmproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) +} +``` + +For those interested in how to incororate these methods into the cosmos-sdk and the app, we do that in the following PRs. + +- celestiaorg/cosmos-sdk [#63](https://github.com/celestiaorg/cosmos-sdk/pull/63) +- celestiaorg/celestia-app [#214](https://github.com/celestiaorg/celestia-app/pull/214) + +### PrepareProposal [#637](https://github.com/celestiaorg/celestia-core/pull/637) and [#224](https://github.com/celestiaorg/celestia-app/pull/224) + +The way that we create proposal blocks will be refactored (previously [`PrePreprocessTxs`](https://github.com/celestiaorg/celestia-app/blob/0363f0410d9d6bf0e51ac92afcaa9be7c0d1ba07/app/abci.go#L17-L108)) to accomodate the new features. + +```go +// PrepareProposal seperates messages from transactions, malleates those transactions, +// estimates the square size, fills the data square, and calculates the data hash from +// the erasure data +func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { + squareSize := app.estimateSquareSize(req.BlockData) + + dataSquare, data, err := WriteSquare(app.txConfig, squareSize, req.BlockData) + if err != nil { + panic(err) + } + + eds, err := da.ExtendShares(squareSize, dataSquare) + if err != nil { + // ... log error + panic(err) + } + + dah := da.NewDataAvailabilityHeader(eds) + data.Hash = dah.Hash() // <-- here we are setting the data hash before we pass the block data back to tendermint + data.OriginalSquareSize = squareSize + + return abci.ResponsePrepareProposal{ + BlockData: data, + } +} +``` + +We estimate the square size by assuming that all of the malleable transaction in the block have a valid commitment for whatever square size that we end up picking, and then quickly iterating through the block data to add up the expected lengths of each message/transaction. Please see [here](https://github.com/celestiaorg/celestia-app/blob/e18d8d2301a96702e1bf684735a3620eb059b12f/app/prepare_proposal.go#L47-L130) for more details. + +In order to efficiently fill the data square and ensure that each message included in the block is paid for, we progressively generate the data square using a few new types. More details can be found in [#637](https://github.com/celestiaorg/celestia-core/pull/637) + +```go +// ContiguousShareWriter lazily merges transaction or other contiguous types in +// the block data into shares that will eventually be included in a data square. +// It also has methods to help progressively count how many shares the transactions +// written take up. +type ContiguousShareWriter struct { + shares []NamespacedShare + pendingShare NamespacedShare + namespace namespace.ID +} +... +// MessageShareWriter lazily merges messages into shares that will eventually be +// included in a data square. It also has methods to help progressively count +// how many shares the messages written take up. +type MessageShareWriter struct { + shares [][]NamespacedShare + count int +} +``` + +These types are combined in a new celestia-app type, `squareWriter`, which is responsible for atomically writing transactions and their corresponding messages to the data square and the returned block data. + +```go +// squareWriter write a data square using provided block data. It also ensures +// that message and their corresponding txs get written to the square +// atomically. +type squareWriter struct { + txWriter *coretypes.ContiguousShareWriter + msgWriter *coretypes.MessageShareWriter + ... +} +``` + +```go +// WriteSquare uses the provided block data to create a flattened data square. +// Any MsgWirePayForMessages are malleated, and their corresponding +// MsgPayForMessage and Message are written atomically. If there are +// transactions that will node fit in the given square size, then they are +// discarded. This is reflected in the returned block data. Note: pointers to +// block data are only used to avoid dereferening, not because we need the block +// data to be mutable. +func WriteSquare(txConf client.TxConfig, squareSize uint64, data *core.Data) ([][]byte, *core.Data, error) { + var ( + processedTxs [][]byte + messages core.Messages + ) + sqwr, err := newSquareWriter(txConf, squareSize, data) + if err != nil { + return nil, nil, err + } + for _, rawTx := range data.Txs { + ... // decode the transaction + + // write the tx to the square if it normal + if !hasWirePayForMessage(authTx) { + success, err := sqwr.writeTx(rawTx) + if err != nil { + continue + } + if !success { + // the square is full + break + } + processedTxs = append(processedTxs, rawTx) + continue + } + + ... + + // attempt to malleate and write the resulting tx + msg to the square + parentHash := sha256.Sum256(rawTx) + success, malTx, message, err := sqwr.writeMalleatedTx(parentHash[:], authTx, wireMsg) + if err != nil { + continue + } + if !success { + // the square is full, but we will attempt to continue to fill the + // block until there are no tx left or no room for txs. While there + // was not room for this particular tx + msg, there might be room + // for other txs or even other smaller messages + continue + } + processedTxs = append(processedTxs, malTx) + messages.MessagesList = append(messages.MessagesList, message) + } + + sort.Slice(messages.MessagesList, func(i, j int) bool { + return bytes.Compare(messages.MessagesList[i].NamespaceId, messages.MessagesList[j].NamespaceId) < 0 + }) + + return sqwr.export(), &core.Data{ + Txs: processedTxs, + Messages: messages, + Evidence: data.Evidence, + IntermediateStateRoots: data.IntermediateStateRoots, + }, nil +} + +... + +// writeTx marshals the tx and lazily writes it to the square. Returns true if +// the write was successful, false if there was not enough room in the square. +func (sqwr *squareWriter) writeTx(tx []byte) (ok bool, err error) { + delimTx, err := coretypes.Tx(tx).MarshalDelimited() + if err != nil { + return false, err + } + + if !sqwr.hasRoomForTx(delimTx) { + return false, nil + } + + sqwr.txWriter.Write(delimTx) + return true, nil +} + +// writeMalleated malleates a MsgWirePayForMessage into a MsgPayForMessage and +// its corresponding message provided that it has a MsgPayForMessage for the +// preselected square size. Returns true if the write was successful, false if +// there was not enough room in the square. +func (sqwr *squareWriter) writeMalleatedTx( + parentHash []byte, + tx signing.Tx, + wpfm *types.MsgWirePayForMessage, +) (ok bool, malleatedTx coretypes.Tx, msg *core.Message, err error) { + ... // process the malleated tx and extract the message. + + // check if we have room for both the tx and message it is crucial that we + // add both atomically, otherwise the block is invalid + if !sqwr.hasRoomForBoth(wrappedTx, coreMsg.Data) { + return false, nil, nil, nil + } + + ... + + sqwr.txWriter.Write(delimTx) + sqwr.msgWriter.Write(coretypes.Message{ + NamespaceID: coreMsg.NamespaceId, + Data: coreMsg.Data, + }) + + return true, wrappedTx, coreMsg, nil +} +``` + +### ProcessProposal [#214](https://github.com/celestiaorg/celestia-app/pull/214), [#216](https://github.com/celestiaorg/celestia-app/pull/216), and [#224](https://github.com/celestiaorg/celestia-app/pull/224) + +During `ProcessProposal`, we + +- quickly compare the commitments found in the transactions that are paying for messages to the commitments generated using the message data contained in the block. +- compare the data hash in the header with that generated in the block data + +```go +func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal { + // Check for message inclusion: + // - each MsgPayForMessage included in a block should have a corresponding message also in the block data + // - the commitment in each PFM should match that of its corresponding message + // - there should be no unpaid for messages + + // extract the commitments from any MsgPayForMessages in the block + commitments := make(map[string]struct{}) + for _, rawTx := range req.BlockData.Txs { + ... + commitments[string(pfm.MessageShareCommitment)] = struct{}{} + ... + } + + // quickly compare the number of PFMs and messages, if they aren't + // identical, then we already know this block is invalid + if len(commitments) != len(req.BlockData.Messages.MessagesList) { + ... // logging and rejecting + } + + ... // generate the data availability header + + if !bytes.Equal(dah.Hash(), req.Header.DataHash) { + ... // logging and rejecting + } + + return abci.ResponseProcessProposal{ + Result: abci.ResponseProcessProposal_ACCEPT, + } +} +``` + +## Status + +Proposed and initial implementation is complete. + +## Consequences + +### Positive +- Don't have to wait for the cosmos-sdk or tendermint teams to finish ABCI++ +- Get to test important features in the upcoming testnet +- We won't have to implement hacky temporary versions of important features. + +### Negative +- We will still have to slightly refactor the code here after ABCI++ comes out + +## References + +### Issues that will be able to be closed after merging this and all implemetation PRs + +[#77](https://github.com/celestiaorg/celestia-core/issues/77) +[#454](https://github.com/celestiaorg/celestia-core/issues/454) +[#520](https://github.com/celestiaorg/celestia-core/issues/520) +[#626](https://github.com/celestiaorg/celestia-core/issues/626) +[#636](https://github.com/celestiaorg/celestia-core/issues/636) + +[#156](https://github.com/celestiaorg/celestia-app/issues/156) +[#204](https://github.com/celestiaorg/celestia-app/issues/204) + +### Open PRs in order of which need to be reviewed/merged first + +[#631](https://github.com/celestiaorg/celestia-core/pull/631) +[#63](https://github.com/celestiaorg/cosmos-sdk/pull/63) +[#214](https://github.com/celestiaorg/celestia-app/pull/214) + +[#216](https://github.com/celestiaorg/celestia-app/pull/216) +[#637](https://github.com/celestiaorg/celestia-core/pull/637) +[#224](https://github.com/celestiaorg/celestia-app/pull/224) + +### Other related by unmerged ADRs that we can close after merging this ADR + +[#559](https://github.com/celestiaorg/celestia-core/pull/559) +[#157](https://github.com/celestiaorg/celestia-app/pull/157) \ No newline at end of file diff --git a/docs/architecture/adr-template.md b/docs/architecture/adr-template.md new file mode 100644 index 0000000000..c7b1e542ae --- /dev/null +++ b/docs/architecture/adr-template.md @@ -0,0 +1,72 @@ +# ADR {ADR-NUMBER}: {TITLE} + +## Changelog + +- {date}: {changelog} + +## Context + +> This section contains all the context one needs to understand the current state, and why there is a problem. It should be as succinct as possible and introduce the high level idea behind the solution. + +## Alternative Approaches + +> This section contains information around alternative options that are considered before making a decision. It should contain a explanation on why the alternative approach(es) were not chosen. + +## Decision + +> This section records the decision that was made. +> It is best to record as much info as possible from the discussion that happened. This aids in not having to go back to the Pull Request to get the needed information. + +## Detailed Design + +> This section does not need to be filled in at the start of the ADR, but must be completed prior to the merging of the implementation. +> +> Here are some common questions that get answered as part of the detailed design: +> +> - What are the user requirements? +> +> - What systems will be affected? +> +> - What new data structures are needed, what data structures will be changed? +> +> - What new APIs will be needed, what APIs will be changed? +> +> - What are the efficiency considerations (time/space)? +> +> - What are the expected access patterns (load/throughput)? +> +> - Are there any logging, monitoring or observability needs? +> +> - Are there any security considerations? +> +> - Are there any privacy considerations? +> +> - How will the changes be tested? +> +> - If the change is large, how will the changes be broken up for ease of review? +> +> - Will these changes require a breaking (major) release? +> +> - Does this change require coordination with the SDK or other? + +## Status + +> A decision may be "proposed" if it hasn't been agreed upon yet, or "accepted" once it is agreed upon. Once the ADR has been implemented mark the ADR as "implemented". If a later ADR changes or reverses a decision, it may be marked as "deprecated" or "superseded" with a reference to its replacement. + +{Deprecated|Proposed|Accepted|Declined} + +## Consequences + +> This section describes the consequences, after applying the decision. All consequences should be summarized here, not just the "positive" ones. + +### Positive + +### Negative + +### Neutral + +## References + +> Are there any relevant PR comments, issues that led up to this, or articles referenced for why we made the given design choice? If so link them here! + +- {reference link} From 2ae506e5e5cefcb53b5a47f67fc95b903dc62831 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 3 Mar 2022 17:47:02 -0600 Subject: [PATCH 02/32] spelling --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 1d91b65430..aaea60928e 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -6,7 +6,7 @@ ## Context -Amongst other things, ABCI++ enables arbitrary logic to be performed by the application to create and verify proposal blocks. +Among other things, ABCI++ enables arbitrary logic to be performed by the application to create and verify proposal blocks. We need this functionality in order for block producers to From 285f756fd13aa7bdaba7882f94634edf41b5d9fe Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 4 Mar 2022 07:47:33 -0600 Subject: [PATCH 03/32] wording and spelling --- docs/architecture/ADR-001-ABCI++.md | 425 ++++++++++++++-------------- 1 file changed, 212 insertions(+), 213 deletions(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index aaea60928e..32fb8e96f6 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -44,50 +44,49 @@ Proposed and initial implementation is complete. ### [#631](https://github.com/celestiaorg/celestia-core/pull/631) Simplified version of ABCI++ (`celestia-core`) -In order to be a viable medium term solution, we want to make as few changes as possible to both the cosmos-sdk and the app. This is done by only incorporating the two necessary new ABCI++ methods that pulled from upstream. Only pulling these two methods from upstream allows for these new methods to be easily be implemented in the cosmos-sdk/app, not unlike we previously added `PreprocessTxs`. `state/execution.go` - +Here we are adding only the two new methods that are necessary for the features that we need. ```go // Application is an interface that enables any finite, deterministic state machine // to be driven by a blockchain-based replication engine via the ABCI. type Application interface { - ... - PrepareProposal(RequestPrepareProposal) ResponsePrepareProposal - ProcessProposal(RequestProcessProposal) ResponseProcessProposal - ... + ... + PrepareProposal(RequestPrepareProposal) ResponsePrepareProposal + ProcessProposal(RequestProcessProposal) ResponseProcessProposal + ... } ``` It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modifed to pass the entire block data. ```protobuf message RequestPrepareProposal { - // block_data is an array of transactions that will be included in a block, - // sent to the app for possible modifications. - // applications can not exceed the size of the data passed to it. - tendermint.types.Data block_data = 1; - // If an application decides to populate block_data with extra information, they can not exceed this value. - int64 block_data_size = 2; + // block_data is an array of transactions that will be included in a block, + // sent to the app for possible modifications. + // applications can not exceed the size of the data passed to it. + tendermint.types.Data block_data = 1; + // If an application decides to populate block_data with extra information, they can not exceed this value. + int64 block_data_size = 2; } message RequestProcessProposal { - tendermint.types.Header header = 1 [(gogoproto.nullable) = false]; - tendermint.types.Data block_data = 2; + tendermint.types.Header header = 1 [(gogoproto.nullable) = false]; + tendermint.types.Data block_data = 2; } ... -// Data contains the set of transactions, evidence, intermediate state roots, +// Data contains the set of transactions, evidence, intermediate state roots, // and messages to be included in the block message Data { - // Txs that will be applied by state @ block.Height+1. - // NOTE: not all txs here are valid. We're just agreeing on the order first. - // This means that block.AppHash does not include these txs. - repeated bytes txs = 1; - - IntermediateStateRoots intermediate_state_roots = 2 [(gogoproto.nullable) = false]; - EvidenceList evidence = 3 [(gogoproto.nullable) = false]; - Messages messages = 4 [(gogoproto.nullable) = false]; - uint64 original_square_size = 5; - bytes hash = 6; // <-- The hash has been added so that the erasure step can be performed in the app + // Txs that will be applied by state @ block.Height+1. + // NOTE: not all txs here are valid. We're just agreeing on the order first. + // This means that block.AppHash does not include these txs. + repeated bytes txs = 1; + + IntermediateStateRoots intermediate_state_roots = 2 [(gogoproto.nullable) = false]; + EvidenceList evidence = 3 [(gogoproto.nullable) = false]; + Messages messages = 4 [(gogoproto.nullable) = false]; + uint64 original_square_size = 5; + bytes hash = 6; // <-- The hash has been added so that the erasure step can be performed in the app } ``` @@ -95,29 +94,29 @@ Here is where the new ABCI++ method is called to perform arbitrary logic in the ```go // CreateProposalBlock calls state.MakeBlock with evidence from the evpool -// and txs from the mempool. +// and txs from the mempool. func (blockExec *BlockExecutor) CreateProposalBlock( - height int64, - state State, commit *types.Commit, - proposerAddr []byte, + height int64, + state State, commit *types.Commit, + proposerAddr []byte, ) (*types.Block, *types.PartSet) { - ... - txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas) - l := len(txs) - bzs := make([][]byte, l) - for i := 0; i < l; i++ { - bzs[i] = txs[i] - } - - preparedProposal, err := blockExec.proxyApp.PrepareProposalSync( - abci.RequestPrepareProposal{ - BlockData: &tmproto.Data{Txs: txs.ToSliceOfBytes(), Evidence: *pevdData}, - BlockDataSize: maxDataBytes}, - ) - ... - return state.MakeBlock( - ... - ) + ... + txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas) + l := len(txs) + bzs := make([][]byte, l) + for i := 0; i < l; i++ { + bzs[i] = txs[i] + } + + preparedProposal, err := blockExec.proxyApp.PrepareProposalSync( + abci.RequestPrepareProposal{ + BlockData: &tmproto.Data{Txs: txs.ToSliceOfBytes(), Evidence: *pevdData}, + BlockDataSize: maxDataBytes}, + ) + ... + return state.MakeBlock( + ... + ) } ``` @@ -125,99 +124,99 @@ Here is where arbitrary logic is performed by the app before voting on a propose ```go func (cs *State) defaultDoPrevote(height int64, round int32) { - ... - stateMachineValidBlock, err := cs.blockExec.ProcessProposal(cs.ProposalBlock) - if err != nil { - cs.Logger.Error("state machine returned an error when trying to process proposal block", "err", err) - } - - // Vote nil if application invalidated the block - if !stateMachineValidBlock { - // Consensus says we must vote nil - logger.Error("prevote step: consensus deems this block to be mustVoteNil", "err", err) - cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) - return - } - - // Prevote cs.ProposalBlock - // NOTE: the proposal signature is validated when it is received, - // and the proposal block parts are validated as they are received (against the merkle hash in the proposal) - logger.Debug("prevote step: ProposalBlock is valid") - cs.signAddVote(tmproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) + ... + stateMachineValidBlock, err := cs.blockExec.ProcessProposal(cs.ProposalBlock) + if err != nil { + cs.Logger.Error("state machine returned an error when trying to process proposal block", "err", err) + } + + // Vote nil if application invalidated the block + if !stateMachineValidBlock { + // Consensus says we must vote nil + logger.Error("prevote step: consensus deems this block to be mustVoteNil", "err", err) + cs.signAddVote(tmproto.PrevoteType, nil, types.PartSetHeader{}) + return + } + + // Prevote cs.ProposalBlock + // NOTE: the proposal signature is validated when it is received, + // and the proposal block parts are validated as they are received (against the merkle hash in the proposal) + logger.Debug("prevote step: ProposalBlock is valid") + cs.signAddVote(tmproto.PrevoteType, cs.ProposalBlock.Hash(), cs.ProposalBlockParts.Header()) } ``` -For those interested in how to incororate these methods into the cosmos-sdk and the app, we do that in the following PRs. +For those interested in how to incorporate these methods into the cosmos-sdk and the app, we do that in the following PRs. - celestiaorg/cosmos-sdk [#63](https://github.com/celestiaorg/cosmos-sdk/pull/63) - celestiaorg/celestia-app [#214](https://github.com/celestiaorg/celestia-app/pull/214) ### PrepareProposal [#637](https://github.com/celestiaorg/celestia-core/pull/637) and [#224](https://github.com/celestiaorg/celestia-app/pull/224) -The way that we create proposal blocks will be refactored (previously [`PrePreprocessTxs`](https://github.com/celestiaorg/celestia-app/blob/0363f0410d9d6bf0e51ac92afcaa9be7c0d1ba07/app/abci.go#L17-L108)) to accomodate the new features. +The way that we create proposal blocks will be refactored (previously [`PrePreprocessTxs`](https://github.com/celestiaorg/celestia-app/blob/0363f0410d9d6bf0e51ac92afcaa9be7c0d1ba07/app/abci.go#L17-L108)) to accommodate the new features. ```go -// PrepareProposal seperates messages from transactions, malleates those transactions, -// estimates the square size, fills the data square, and calculates the data hash from +// PrepareProposal separates messages from transactions, malleates those transactions, +// estimates the square size, fills the data square, and calculates the data hash from // the erasure data func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - squareSize := app.estimateSquareSize(req.BlockData) - - dataSquare, data, err := WriteSquare(app.txConfig, squareSize, req.BlockData) - if err != nil { - panic(err) - } - - eds, err := da.ExtendShares(squareSize, dataSquare) - if err != nil { - // ... log error - panic(err) - } - - dah := da.NewDataAvailabilityHeader(eds) - data.Hash = dah.Hash() // <-- here we are setting the data hash before we pass the block data back to tendermint - data.OriginalSquareSize = squareSize - - return abci.ResponsePrepareProposal{ - BlockData: data, - } + squareSize := app.estimateSquareSize(req.BlockData) + + dataSquare, data, err := WriteSquare(app.txConfig, squareSize, req.BlockData) + if err != nil { + panic(err) + } + + eds, err := da.ExtendShares(squareSize, dataSquare) + if err != nil { + // ... log error + panic(err) + } + + dah := da.NewDataAvailabilityHeader(eds) + data.Hash = dah.Hash() // <-- here we are setting the data hash before we pass the block data back to tendermint + data.OriginalSquareSize = squareSize + + return abci.ResponsePrepareProposal{ + BlockData: data, + } } ``` -We estimate the square size by assuming that all of the malleable transaction in the block have a valid commitment for whatever square size that we end up picking, and then quickly iterating through the block data to add up the expected lengths of each message/transaction. Please see [here](https://github.com/celestiaorg/celestia-app/blob/e18d8d2301a96702e1bf684735a3620eb059b12f/app/prepare_proposal.go#L47-L130) for more details. +We estimate the square size by assuming that all of the malleable transactions in the block have a valid commitment for whatever square size that we end up picking, and then quickly iterating through the block data to add up the expected lengths of each message/transaction. Please see [here](https://github.com/celestiaorg/celestia-app/blob/e18d8d2301a96702e1bf684735a3620eb059b12f/app/prepare_proposal.go#L47-L130) for more details. In order to efficiently fill the data square and ensure that each message included in the block is paid for, we progressively generate the data square using a few new types. More details can be found in [#637](https://github.com/celestiaorg/celestia-core/pull/637) ```go -// ContiguousShareWriter lazily merges transaction or other contiguous types in -// the block data into shares that will eventually be included in a data square. -// It also has methods to help progressively count how many shares the transactions +// ContiguousShareWriter lazily merges transaction or other contiguous types in +// the block data into shares that will eventually be included in a data square. +// It also has methods to help progressively count how many shares the transactions // written take up. type ContiguousShareWriter struct { - shares []NamespacedShare - pendingShare NamespacedShare - namespace namespace.ID + shares []NamespacedShare + pendingShare NamespacedShare + namespace namespace.ID } ... // MessageShareWriter lazily merges messages into shares that will eventually be // included in a data square. It also has methods to help progressively count // how many shares the messages written take up. type MessageShareWriter struct { - shares [][]NamespacedShare - count int + shares [][]NamespacedShare + count int } ``` These types are combined in a new celestia-app type, `squareWriter`, which is responsible for atomically writing transactions and their corresponding messages to the data square and the returned block data. ```go -// squareWriter write a data square using provided block data. It also ensures +// squareWriter writes a data square using provided block data. It also ensures // that message and their corresponding txs get written to the square // atomically. type squareWriter struct { - txWriter *coretypes.ContiguousShareWriter - msgWriter *coretypes.MessageShareWriter - ... + txWriter *coretypes.ContiguousShareWriter + msgWriter *coretypes.MessageShareWriter + ... } ``` @@ -227,63 +226,63 @@ type squareWriter struct { // MsgPayForMessage and Message are written atomically. If there are // transactions that will node fit in the given square size, then they are // discarded. This is reflected in the returned block data. Note: pointers to -// block data are only used to avoid dereferening, not because we need the block +// block data are only used to avoid dereferencing, not because we need the block // data to be mutable. func WriteSquare(txConf client.TxConfig, squareSize uint64, data *core.Data) ([][]byte, *core.Data, error) { - var ( - processedTxs [][]byte - messages core.Messages - ) - sqwr, err := newSquareWriter(txConf, squareSize, data) - if err != nil { - return nil, nil, err - } - for _, rawTx := range data.Txs { - ... // decode the transaction - - // write the tx to the square if it normal - if !hasWirePayForMessage(authTx) { - success, err := sqwr.writeTx(rawTx) - if err != nil { - continue - } - if !success { - // the square is full - break - } - processedTxs = append(processedTxs, rawTx) - continue - } - - ... - - // attempt to malleate and write the resulting tx + msg to the square - parentHash := sha256.Sum256(rawTx) - success, malTx, message, err := sqwr.writeMalleatedTx(parentHash[:], authTx, wireMsg) - if err != nil { - continue - } - if !success { - // the square is full, but we will attempt to continue to fill the - // block until there are no tx left or no room for txs. While there - // was not room for this particular tx + msg, there might be room - // for other txs or even other smaller messages - continue - } - processedTxs = append(processedTxs, malTx) - messages.MessagesList = append(messages.MessagesList, message) - } - - sort.Slice(messages.MessagesList, func(i, j int) bool { - return bytes.Compare(messages.MessagesList[i].NamespaceId, messages.MessagesList[j].NamespaceId) < 0 - }) - - return sqwr.export(), &core.Data{ - Txs: processedTxs, - Messages: messages, - Evidence: data.Evidence, - IntermediateStateRoots: data.IntermediateStateRoots, - }, nil + var ( + processedTxs [][]byte + messages core.Messages + ) + sqwr, err := newSquareWriter(txConf, squareSize, data) + if err != nil { + return nil, nil, err + } + for _, rawTx := range data.Txs { + ... // decode the transaction + + // write the tx to the square if it normal + if !hasWirePayForMessage(authTx) { + success, err := sqwr.writeTx(rawTx) + if err != nil { + continue + } + if !success { + // the square is full + break + } + processedTxs = append(processedTxs, rawTx) + continue + } + + ... + + // attempt to malleate and write the resulting tx + msg to the square + parentHash := sha256.Sum256(rawTx) + success, malTx, message, err := sqwr.writeMalleatedTx(parentHash[:], authTx, wireMsg) + if err != nil { + continue + } + if !success { + // the square is full, but we will attempt to continue to fill the + // block until there are no tx left or no room for txs. While there + // was not room for this particular tx + msg, there might be room + // for other txs or even other smaller messages + continue + } + processedTxs = append(processedTxs, malTx) + messages.MessagesList = append(messages.MessagesList, message) + } + + sort.Slice(messages.MessagesList, func(i, j int) bool { + return bytes.Compare(messages.MessagesList[i].NamespaceId, messages.MessagesList[j].NamespaceId) < 0 + }) + + return sqwr.export(), &core.Data{ + Txs: processedTxs, + Messages: messages, + Evidence: data.Evidence, + IntermediateStateRoots: data.IntermediateStateRoots, + }, nil } ... @@ -291,17 +290,17 @@ func WriteSquare(txConf client.TxConfig, squareSize uint64, data *core.Data) ([] // writeTx marshals the tx and lazily writes it to the square. Returns true if // the write was successful, false if there was not enough room in the square. func (sqwr *squareWriter) writeTx(tx []byte) (ok bool, err error) { - delimTx, err := coretypes.Tx(tx).MarshalDelimited() - if err != nil { - return false, err - } + delimTx, err := coretypes.Tx(tx).MarshalDelimited() + if err != nil { + return false, err + } - if !sqwr.hasRoomForTx(delimTx) { - return false, nil - } + if !sqwr.hasRoomForTx(delimTx) { + return false, nil + } - sqwr.txWriter.Write(delimTx) - return true, nil + sqwr.txWriter.Write(delimTx) + return true, nil } // writeMalleated malleates a MsgWirePayForMessage into a MsgPayForMessage and @@ -309,27 +308,27 @@ func (sqwr *squareWriter) writeTx(tx []byte) (ok bool, err error) { // preselected square size. Returns true if the write was successful, false if // there was not enough room in the square. func (sqwr *squareWriter) writeMalleatedTx( - parentHash []byte, - tx signing.Tx, - wpfm *types.MsgWirePayForMessage, + parentHash []byte, + tx signing.Tx, + wpfm *types.MsgWirePayForMessage, ) (ok bool, malleatedTx coretypes.Tx, msg *core.Message, err error) { - ... // process the malleated tx and extract the message. + ... // process the malleated tx and extract the message. - // check if we have room for both the tx and message it is crucial that we - // add both atomically, otherwise the block is invalid - if !sqwr.hasRoomForBoth(wrappedTx, coreMsg.Data) { - return false, nil, nil, nil - } + // check if we have room for both the tx and message it is crucial that we + // add both atomically, otherwise the block is invalid + if !sqwr.hasRoomForBoth(wrappedTx, coreMsg.Data) { + return false, nil, nil, nil + } - ... + ... - sqwr.txWriter.Write(delimTx) - sqwr.msgWriter.Write(coretypes.Message{ - NamespaceID: coreMsg.NamespaceId, - Data: coreMsg.Data, - }) + sqwr.txWriter.Write(delimTx) + sqwr.msgWriter.Write(coretypes.Message{ + NamespaceID: coreMsg.NamespaceId, + Data: coreMsg.Data, + }) - return true, wrappedTx, coreMsg, nil + return true, wrappedTx, coreMsg, nil } ``` @@ -342,34 +341,34 @@ During `ProcessProposal`, we ```go func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal { - // Check for message inclusion: - // - each MsgPayForMessage included in a block should have a corresponding message also in the block data - // - the commitment in each PFM should match that of its corresponding message - // - there should be no unpaid for messages - - // extract the commitments from any MsgPayForMessages in the block - commitments := make(map[string]struct{}) - for _, rawTx := range req.BlockData.Txs { - ... - commitments[string(pfm.MessageShareCommitment)] = struct{}{} - ... - } - - // quickly compare the number of PFMs and messages, if they aren't - // identical, then we already know this block is invalid - if len(commitments) != len(req.BlockData.Messages.MessagesList) { - ... // logging and rejecting - } - - ... // generate the data availability header - - if !bytes.Equal(dah.Hash(), req.Header.DataHash) { - ... // logging and rejecting - } - - return abci.ResponseProcessProposal{ - Result: abci.ResponseProcessProposal_ACCEPT, - } + // Check for message inclusion: + // - each MsgPayForMessage included in a block should have a corresponding message also in the block data + // - the commitment in each PFM should match that of its corresponding message + // - there should be no unpaid for messages + + // extract the commitments from any MsgPayForMessages in the block + commitments := make(map[string]struct{}) + for _, rawTx := range req.BlockData.Txs { + ... + commitments[string(pfm.MessageShareCommitment)] = struct{}{} + ... + } + + // quickly compare the number of PFMs and messages, if they aren't + // identical, then we already know this block is invalid + if len(commitments) != len(req.BlockData.Messages.MessagesList) { + ... // logging and rejecting + } + + ... // generate the data availability header + + if !bytes.Equal(dah.Hash(), req.Header.DataHash) { + ... // logging and rejecting + } + + return abci.ResponseProcessProposal{ + Result: abci.ResponseProcessProposal_ACCEPT, + } } ``` @@ -385,11 +384,11 @@ Proposed and initial implementation is complete. - We won't have to implement hacky temporary versions of important features. ### Negative -- We will still have to slightly refactor the code here after ABCI++ comes out +- We will still have to slightly refactor the code here after ABCI++ comes out ## References -### Issues that will be able to be closed after merging this and all implemetation PRs +### Issues that will be able to be closed after merging this and all implementation PRs [#77](https://github.com/celestiaorg/celestia-core/issues/77) [#454](https://github.com/celestiaorg/celestia-core/issues/454) @@ -413,4 +412,4 @@ Proposed and initial implementation is complete. ### Other related by unmerged ADRs that we can close after merging this ADR [#559](https://github.com/celestiaorg/celestia-core/pull/559) -[#157](https://github.com/celestiaorg/celestia-app/pull/157) \ No newline at end of file +[#157](https://github.com/celestiaorg/celestia-app/pull/157) From bb82fa55eebce8d3e2d9abd6615925aed33b7126 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 4 Mar 2022 08:28:30 -0600 Subject: [PATCH 04/32] change data format --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 32fb8e96f6..083c54c051 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -2,7 +2,7 @@ ## Changelog -- 03/03/2022: Initial Commit +- 2022-03-03: Initial Commit ## Context From 002b24b9b1c14cee0ac77fe5669f6e1ccc0813f1 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 4 Mar 2022 12:29:28 -0600 Subject: [PATCH 05/32] more accurate statement --- docs/architecture/ADR-001-ABCI++.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 083c54c051..74bf5d1c0a 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -19,8 +19,7 @@ We need this functionality in order for block producers to We also need this functionality for validators to verify that -- Messages that are paid for by users are actually included in the block -- The block producer doesn’t also include messages that are not paid for +- For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. - That the data hash represents the properly erasured block data for the selected block size - Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) From fda5e002704f781624ecc0c7effea0d4fd336b03 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 7 Mar 2022 12:59:18 -0600 Subject: [PATCH 06/32] wording Co-authored-by: John Adler --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 74bf5d1c0a..fd72000fb9 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -15,7 +15,7 @@ We need this functionality in order for block producers to - Separate rollup block data from the stateful portion of the transaction - Use the appropriate signature for the selected block size - Follow the non-interactive default rules (not done here) -- Create celestia specific data hash by erasuring the block data +- Create Celestia-specific data hash by erasure-coding the block data We also need this functionality for validators to verify that From 52d35b1ed848e1d2aca80bba8c01df69a753cf6d Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 7 Mar 2022 12:59:50 -0600 Subject: [PATCH 07/32] typo Co-authored-by: John Adler --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index fd72000fb9..d9b034ea8e 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -10,7 +10,7 @@ Among other things, ABCI++ enables arbitrary logic to be performed by the applic We need this functionality in order for block producers to -- Pick a square size / Fill the data square efficiently +- Pick a square size / fill the data square efficiently - Malleate user transactions - Separate rollup block data from the stateful portion of the transaction - Use the appropriate signature for the selected block size From 076bc67f12aab253cd4fc86f377337cf48c9c8b3 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 7 Mar 2022 13:00:08 -0600 Subject: [PATCH 08/32] erasured is not a word Co-authored-by: John Adler --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index d9b034ea8e..7127a05b7f 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -20,7 +20,7 @@ We need this functionality in order for block producers to We also need this functionality for validators to verify that - For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. -- That the data hash represents the properly erasured block data for the selected block size +- That the data hash represents the properly-erasure-coded block data for the selected block size. - Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the work for us, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by creating a simplified version of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. From b6e97370fa005317e180991196a33f1c57d251c6 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 9 Mar 2022 09:29:24 -0600 Subject: [PATCH 09/32] typo Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 7127a05b7f..1b71fa396c 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -55,7 +55,7 @@ type Application interface { } ``` -It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modifed to pass the entire block data. +It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modified to pass the entire block data. ```protobuf message RequestPrepareProposal { // block_data is an array of transactions that will be included in a block, From c8bc16d87db9fada1f4e68a0fb14e366a3a76397 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 9 Mar 2022 09:31:15 -0600 Subject: [PATCH 10/32] wording Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 1b71fa396c..3b5d294770 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -182,7 +182,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr } ``` -We estimate the square size by assuming that all of the malleable transactions in the block have a valid commitment for whatever square size that we end up picking, and then quickly iterating through the block data to add up the expected lengths of each message/transaction. Please see [here](https://github.com/celestiaorg/celestia-app/blob/e18d8d2301a96702e1bf684735a3620eb059b12f/app/prepare_proposal.go#L47-L130) for more details. +We estimate the square size by assuming that all the malleable transactions in the block have a valid commitment for whatever square size that we end up picking, and then quickly iterating through the block data to add up the expected lengths of each message/transaction. Please see [here](https://github.com/celestiaorg/celestia-app/blob/e18d8d2301a96702e1bf684735a3620eb059b12f/app/prepare_proposal.go#L47-L130) for more details. In order to efficiently fill the data square and ensure that each message included in the block is paid for, we progressively generate the data square using a few new types. More details can be found in [#637](https://github.com/celestiaorg/celestia-core/pull/637) From 809e46e03f841dfaeea3b84ea8e2956cb4365157 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Wed, 16 Mar 2022 20:16:31 -0500 Subject: [PATCH 11/32] add link to non-interactive defaults --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 3b5d294770..df26421321 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -14,7 +14,7 @@ We need this functionality in order for block producers to - Malleate user transactions - Separate rollup block data from the stateful portion of the transaction - Use the appropriate signature for the selected block size -- Follow the non-interactive default rules (not done here) +- Follow the [non-interactive default rules](https://github.com/celestiaorg/celestia-specs/blob/master/src/rationale/message_block_layout.md#non-interactive-default-rules) (not done here) - Create Celestia-specific data hash by erasure-coding the block data We also need this functionality for validators to verify that From 5fcaec13827b0c4f3696471cfcd615c5005db4ef Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 16 Mar 2022 20:17:21 -0500 Subject: [PATCH 12/32] better wording Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 3b5d294770..c7176c5b90 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -23,7 +23,7 @@ We also need this functionality for validators to verify that - That the data hash represents the properly-erasure-coded block data for the selected block size. - Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) -Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the work for us, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by creating a simplified version of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. +Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the heavy lifting, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by using a subset of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. ## Alternative Approaches From 4658a605876f4e78430654ad308f573c30c646a9 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 16 Mar 2022 20:17:51 -0500 Subject: [PATCH 13/32] Update docs/architecture/ADR-001-ABCI++.md Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index c7176c5b90..31849e39b9 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -223,7 +223,7 @@ type squareWriter struct { // WriteSquare uses the provided block data to create a flattened data square. // Any MsgWirePayForMessages are malleated, and their corresponding // MsgPayForMessage and Message are written atomically. If there are -// transactions that will node fit in the given square size, then they are +// transactions that will not fit in the given square size, then they are // discarded. This is reflected in the returned block data. Note: pointers to // block data are only used to avoid dereferencing, not because we need the block // data to be mutable. From c4d83d9881d57b8328d52846baa985733ecb527f Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 16 Mar 2022 20:18:10 -0500 Subject: [PATCH 14/32] correct docs Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 31849e39b9..6f63be1289 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -302,7 +302,7 @@ func (sqwr *squareWriter) writeTx(tx []byte) (ok bool, err error) { return true, nil } -// writeMalleated malleates a MsgWirePayForMessage into a MsgPayForMessage and +// writeMalleatedTx malleates a MsgWirePayForMessage into a MsgPayForMessage and // its corresponding message provided that it has a MsgPayForMessage for the // preselected square size. Returns true if the write was successful, false if // there was not enough room in the square. From f7b6fcb7ba324d75f52a484585090c5844ca8a22 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Wed, 16 Mar 2022 20:25:05 -0500 Subject: [PATCH 15/32] rename writeSquare to SplitShares --- docs/architecture/ADR-001-ABCI++.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index df26421321..1c7396d3fb 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -161,7 +161,7 @@ The way that we create proposal blocks will be refactored (previously [`PrePrepr func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { squareSize := app.estimateSquareSize(req.BlockData) - dataSquare, data, err := WriteSquare(app.txConfig, squareSize, req.BlockData) + dataSquare, data, err := SplitShares(app.txConfig, squareSize, req.BlockData) if err != nil { panic(err) } @@ -220,14 +220,14 @@ type squareWriter struct { ``` ```go -// WriteSquare uses the provided block data to create a flattened data square. +// SplitShares uses the provided block data to create a flattened data square. // Any MsgWirePayForMessages are malleated, and their corresponding // MsgPayForMessage and Message are written atomically. If there are // transactions that will node fit in the given square size, then they are // discarded. This is reflected in the returned block data. Note: pointers to // block data are only used to avoid dereferencing, not because we need the block // data to be mutable. -func WriteSquare(txConf client.TxConfig, squareSize uint64, data *core.Data) ([][]byte, *core.Data, error) { +func SplitShares(txConf client.TxConfig, squareSize uint64, data *core.Data) ([][]byte, *core.Data, error) { var ( processedTxs [][]byte messages core.Messages From 5b987871eeed9bd2a767a846dc3ccc06be781647 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Fri, 1 Apr 2022 14:39:31 -0500 Subject: [PATCH 16/32] use specific commit Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 225585552f..b4fab1abef 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -14,7 +14,7 @@ We need this functionality in order for block producers to - Malleate user transactions - Separate rollup block data from the stateful portion of the transaction - Use the appropriate signature for the selected block size -- Follow the [non-interactive default rules](https://github.com/celestiaorg/celestia-specs/blob/master/src/rationale/message_block_layout.md#non-interactive-default-rules) (not done here) +- Follow the [non-interactive default rules](https://github.com/celestiaorg/celestia-specs/blob/65d4d528beafaf8d84f7eb805db940f05f2a2c93/src/rationale/message_block_layout.md#non-interactive-default-rules) (not done here) - Create Celestia-specific data hash by erasure-coding the block data We also need this functionality for validators to verify that From c9cb6063c939ac82b0a999f01f80292e6c19a2d5 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 1 Apr 2022 14:53:32 -0500 Subject: [PATCH 17/32] add explainer sentance for why all block data has to be passed --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 225585552f..51c5f827b6 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -55,7 +55,7 @@ type Application interface { } ``` -It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modified to pass the entire block data. +It's also important to note the changes made to the request types for both methods. In upstream, they are only passing the transactions to the applications. This has been modified to pass the entire block data. This is because Celestia separates some block data that cannot modify state (messages), and the application has to have access to both normal transaction data and messages to perform the necessary processing and checks. ```protobuf message RequestPrepareProposal { // block_data is an array of transactions that will be included in a block, From 40861a89e557e23c301e772ca8420440dbe366eb Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 1 Apr 2022 15:12:17 -0500 Subject: [PATCH 18/32] explainer paragraph for setting the data hash in the app --- docs/architecture/ADR-001-ABCI++.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 51c5f827b6..9501c58e43 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -331,6 +331,8 @@ func (sqwr *squareWriter) writeMalleatedTx( } ``` +Lastly, the data availability header is used to create the `DataHash` in the `Header` in the application instead of in tendermint. This is done by modifying the protobuf version of the block data to retain the cached hash and setting it during `ProcessProposal`. Later, in `ProcessProposal` other fullnodes check that the `DataHash` matches the block data by recomputing it. Previously, this extra check was performed inside the `ValidateBasic` method of `types.Data`, where is was computed each time it was decoded. Not only is this more efficient as it saves significant computational resources and keeps `ValidateBasic` light, it is also much more explicit. This approach does not however dramatically any existing code in tendermint, as the code to compute the hash of the block data remains there. Ideally, we would move all of the code that computes erasure encoding to the app. This approach allows us to keep the intuitiveness of the `Hash` method for `types.Data`, along with not forcing us to change many tests in tendermint, which rely on this functionality. + ### ProcessProposal [#214](https://github.com/celestiaorg/celestia-app/pull/214), [#216](https://github.com/celestiaorg/celestia-app/pull/216), and [#224](https://github.com/celestiaorg/celestia-app/pull/224) During `ProcessProposal`, we From 0b0639cb2035006a26a4a3d14970c989fe26255a Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:04:46 -0500 Subject: [PATCH 19/32] use colon Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 4f09ac6ebb..b3882ee31d 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -8,7 +8,7 @@ Among other things, ABCI++ enables arbitrary logic to be performed by the application to create and verify proposal blocks. -We need this functionality in order for block producers to +We need this functionality in order for block producers to: - Pick a square size / fill the data square efficiently - Malleate user transactions From 23efeaa714680c151fcb376aafa995d3bb6bd637 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:05:03 -0500 Subject: [PATCH 20/32] use colon Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index b3882ee31d..03e41c938c 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -17,7 +17,7 @@ We need this functionality in order for block producers to: - Follow the [non-interactive default rules](https://github.com/celestiaorg/celestia-specs/blob/65d4d528beafaf8d84f7eb805db940f05f2a2c93/src/rationale/message_block_layout.md#non-interactive-default-rules) (not done here) - Create Celestia-specific data hash by erasure-coding the block data -We also need this functionality for validators to verify that +We also need this functionality for validators to verify that: - For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. - That the data hash represents the properly-erasure-coded block data for the selected block size. From 99f29779469bd2793a2c111aac3da3b352a5fbbe Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:05:23 -0500 Subject: [PATCH 21/32] better wording Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 03e41c938c..35a8ddaf61 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -20,7 +20,7 @@ We need this functionality in order for block producers to: We also need this functionality for validators to verify that: - For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. -- That the data hash represents the properly-erasure-coded block data for the selected block size. +- The data hash represents the properly-erasure-coded block data for the selected block size. - Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the heavy lifting, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by using a subset of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. From 40f7f74e994cff23b0838ac5f6e444dfb0a020d2 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:05:39 -0500 Subject: [PATCH 22/32] better wording Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 35a8ddaf61..54d854dd75 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -21,7 +21,7 @@ We also need this functionality for validators to verify that: - For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. - The data hash represents the properly-erasure-coded block data for the selected block size. -- Included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) +- The included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) Technically, we don’t have to use ABCI++ yet, we could still test some needed features in the upcoming testnet without it. However, these implementations would not be representative of the implementations that would actually make it to mainnet, as they would have to be significantly different from their ABCI++ counterparts. The decision to adopt ABCI++ earlier becomes easier considering that the tendermint team has already done most of the heavy lifting, and it is possible to start working on the needed features without waiting for the cosmos-sdk team to use them. We explain our plans below to do just this, by using a subset of ABCI++ (ABCI+?) using only the new methods that are necessary, finished, and easy to incorporate into the cosmos-sdk. From df01138b48a42b4e7c39ab5119fb1705adb334e1 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:06:26 -0500 Subject: [PATCH 23/32] embed link Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 54d854dd75..1b3554fdbc 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -29,7 +29,7 @@ Technically, we don’t have to use ABCI++ yet, we could still test some needed While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++. -Alternatives for Message Inclusion +- [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches Alternatives for Picking a square size From 44629a8754a936794431ef3006668858acbcbd9c Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:06:51 -0500 Subject: [PATCH 24/32] embed link Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 1b3554fdbc..c104f3f9f5 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -32,7 +32,7 @@ While the adoption of ABCI++ is inevitable given the already made decision by up - [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches -Alternatives for Picking a square size +- [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454) https://github.com/celestiaorg/celestia-core/issues/454 ## Decision From 94312712802cf4bb7230158dc863e3f21843da86 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:07:13 -0500 Subject: [PATCH 25/32] remove non-embedded link Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index c104f3f9f5..65d71c2a6e 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -30,7 +30,6 @@ Technically, we don’t have to use ABCI++ yet, we could still test some needed While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++. - [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) -https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches - [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454) https://github.com/celestiaorg/celestia-core/issues/454 From 6f8432893e9d1dc2da2665b9492d5dc9f0140c05 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:07:32 -0500 Subject: [PATCH 26/32] remove non-embedded link Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 65d71c2a6e..16c574de12 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -32,7 +32,6 @@ While the adoption of ABCI++ is inevitable given the already made decision by up - [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) - [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454) -https://github.com/celestiaorg/celestia-core/issues/454 ## Decision From 39e10ec653b848a5392f3e85b60f1296bef55ae8 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:07:50 -0500 Subject: [PATCH 27/32] remove extra line Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 16c574de12..dd84b5e634 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -30,7 +30,6 @@ Technically, we don’t have to use ABCI++ yet, we could still test some needed While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++. - [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) - - [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454) ## Decision From 32b3a9ec29950bd0406c1839613a230b8a75d0ae Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Wed, 6 Apr 2022 09:36:02 -0500 Subject: [PATCH 28/32] typo Co-authored-by: CHAMI Rachid --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index dd84b5e634..6ab8f1043a 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -27,7 +27,7 @@ Technically, we don’t have to use ABCI++ yet, we could still test some needed ## Alternative Approaches -While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++. +While the adoption of ABCI++ is inevitable given the already made decision by upstream to implement it, here are some alternatives to the features that we need that do not use ABCI++: - [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches) - [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454) From 33b8dab7cbf0cbdac526de3ba06f94bacb229665 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Sun, 10 Apr 2022 10:48:40 -0500 Subject: [PATCH 29/32] change to MsgPayForData Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 6ab8f1043a..47ce8f98d9 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -19,7 +19,7 @@ We need this functionality in order for block producers to: We also need this functionality for validators to verify that: -- For every `MsgPayForMessage` included in the block, there is also a corresponding message and vice versa. +- For every `MsgPayForData` (previously `MsgPayForMessage`) included in the block, there is also a corresponding message and vice versa. - The data hash represents the properly-erasure-coded block data for the selected block size. - The included messages are arranged in the expected locations in the square according to the non-interactive default rules (not done here) From bc18b5e750e6fddfa58b5eb987acf533a49fe2c4 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Sun, 10 Apr 2022 10:49:29 -0500 Subject: [PATCH 30/32] remove isrs from the adr Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 47ce8f98d9..73ad4138b7 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -70,7 +70,7 @@ message RequestProcessProposal { ... -// Data contains the set of transactions, evidence, intermediate state roots, +// Data contains the set of transactions, evidence, // and messages to be included in the block message Data { // Txs that will be applied by state @ block.Height+1. From c95018f80488848d5d33bd915c1b2c76a1471cd0 Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Sun, 10 Apr 2022 10:50:42 -0500 Subject: [PATCH 31/32] include word change in sentence Co-authored-by: Ismail Khoffi --- docs/architecture/ADR-001-ABCI++.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 73ad4138b7..97153387c6 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -328,7 +328,7 @@ func (sqwr *squareWriter) writeMalleatedTx( } ``` -Lastly, the data availability header is used to create the `DataHash` in the `Header` in the application instead of in tendermint. This is done by modifying the protobuf version of the block data to retain the cached hash and setting it during `ProcessProposal`. Later, in `ProcessProposal` other fullnodes check that the `DataHash` matches the block data by recomputing it. Previously, this extra check was performed inside the `ValidateBasic` method of `types.Data`, where is was computed each time it was decoded. Not only is this more efficient as it saves significant computational resources and keeps `ValidateBasic` light, it is also much more explicit. This approach does not however dramatically any existing code in tendermint, as the code to compute the hash of the block data remains there. Ideally, we would move all of the code that computes erasure encoding to the app. This approach allows us to keep the intuitiveness of the `Hash` method for `types.Data`, along with not forcing us to change many tests in tendermint, which rely on this functionality. +Lastly, the data availability header is used to create the `DataHash` in the `Header` in the application instead of in tendermint. This is done by modifying the protobuf version of the block data to retain the cached hash and setting it during `ProcessProposal`. Later, in `ProcessProposal` other fullnodes check that the `DataHash` matches the block data by recomputing it. Previously, this extra check was performed inside the `ValidateBasic` method of `types.Data`, where is was computed each time it was decoded. Not only is this more efficient as it saves significant computational resources and keeps `ValidateBasic` light, it is also much more explicit. This approach does not however dramatically change any existing code in tendermint, as the code to compute the hash of the block data remains there. Ideally, we would move all of the code that computes erasure encoding to the app. This approach allows us to keep the intuitiveness of the `Hash` method for `types.Data`, along with not forcing us to change many tests in tendermint, which rely on this functionality. ### ProcessProposal [#214](https://github.com/celestiaorg/celestia-app/pull/214), [#216](https://github.com/celestiaorg/celestia-app/pull/216), and [#224](https://github.com/celestiaorg/celestia-app/pull/224) From 9dace036b5c51076f73e8c38699b5360b11ee3a1 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Sun, 10 Apr 2022 17:53:17 -0500 Subject: [PATCH 32/32] remove intermediate state roots --- docs/architecture/ADR-001-ABCI++.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/architecture/ADR-001-ABCI++.md b/docs/architecture/ADR-001-ABCI++.md index 97153387c6..6cb9418088 100644 --- a/docs/architecture/ADR-001-ABCI++.md +++ b/docs/architecture/ADR-001-ABCI++.md @@ -78,7 +78,6 @@ message Data { // This means that block.AppHash does not include these txs. repeated bytes txs = 1; - IntermediateStateRoots intermediate_state_roots = 2 [(gogoproto.nullable) = false]; EvidenceList evidence = 3 [(gogoproto.nullable) = false]; Messages messages = 4 [(gogoproto.nullable) = false]; uint64 original_square_size = 5; @@ -277,7 +276,6 @@ func SplitShares(txConf client.TxConfig, squareSize uint64, data *core.Data) ([] Txs: processedTxs, Messages: messages, Evidence: data.Evidence, - IntermediateStateRoots: data.IntermediateStateRoots, }, nil }