From 716ffa5ee4e257f0683e17a9b61870ba480fd6dd Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 06:04:31 +0100 Subject: [PATCH 01/30] feat(utility): setProposalBlock should be called before apply --- utility/types/error.go | 8 +++++++- utility/unit_of_work/block_test.go | 7 ++++++- utility/unit_of_work/module.go | 8 ++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/utility/types/error.go b/utility/types/error.go index 6ede51b32..122268625 100644 --- a/utility/types/error.go +++ b/utility/types/error.go @@ -40,7 +40,7 @@ func NewError(code Code, msg string) Error { } } -// NextCode: 133 +// NextCode: 134 type Code float64 // CONSIDERATION: Should these be a proto enum or a golang iota? //nolint:gosec // G101 - Not hard-coded credentials @@ -174,6 +174,7 @@ const ( CodeGetHeightError Code = 129 CodeUnknownActorType Code = 130 CodeUnknownMessageType Code = 131 + CodeProposalBlockNotSet Code = 133 ) const ( @@ -284,6 +285,7 @@ const ( InvalidTransactionCountError = "the total transactions are less than the block transactions" EmptyTimestampError = "the timestamp field is empty" EmptyProposerError = "the proposer field is empty" + ProposalBlockNotSet = "the proposal block is not set" EmptyNetworkIDError = "the network id field is empty" InvalidHashLengthError = "the length of the hash is not the correct size" NilQuorumCertificateError = "the quorum certificate is nil" @@ -767,6 +769,10 @@ func ErrEmptyProposer() Error { return NewError(CodeEmptyProposerError, EmptyProposerError) } +func ErrProposalBlockNotSet() Error { + return NewError(CodeProposalBlockNotSet, ProposalBlockNotSet) +} + func ErrEmptyTimestamp() Error { return NewError(CodeEmptyTimestampError, EmptyTimestampError) } diff --git a/utility/unit_of_work/block_test.go b/utility/unit_of_work/block_test.go index 5cf17551c..9ccfa1044 100644 --- a/utility/unit_of_work/block_test.go +++ b/utility/unit_of_work/block_test.go @@ -9,6 +9,7 @@ import ( coreTypes "github.com/pokt-network/pocket/shared/core/types" "github.com/pokt-network/pocket/shared/modules" mockModules "github.com/pokt-network/pocket/shared/modules/mocks" + utilTypes "github.com/pokt-network/pocket/utility/types" "github.com/stretchr/testify/require" ) @@ -35,7 +36,11 @@ func TestUtilityUnitOfWork_ApplyBlock(t *testing.T) { proposerBeforeBalance, err := uow.getAccountAmount(addrBz) require.NoError(t, err) - err = uow.SetProposalBlock("", addrBz, [][]byte{txBz}) + // calling ApplyBlock without having called SetProposalBlock first should fail with ErrProposalBlockNotSet + _, _, err = uow.ApplyBlock() + require.Equal(t, err.Error(), utilTypes.ErrProposalBlockNotSet().Error()) + + err = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) require.NoError(t, err) appHash, _, err := uow.ApplyBlock() diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index e01818b96..7c9bb30e0 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -122,6 +122,10 @@ func (u *baseUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, max // CLEANUP: code re-use ApplyBlock() for CreateAndApplyBlock() func (u *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, err error) { + if !u.isProposalBlockSet() { + return "", nil, utilTypes.ErrProposalBlockNotSet() + } + lastByzantineValidators, err := u.prevBlockByzantineValidators() if err != nil { return "", nil, err @@ -216,3 +220,7 @@ func (uow *baseUtilityUnitOfWork) Release() error { return nil } + +func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { + return uow.proposalStateHash != "" && uow.proposalProposerAddr != nil && uow.proposalBlockTxs != nil +} From f7e21f4e19beb8448463203ca45ca1701385114e Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 14:54:00 +0100 Subject: [PATCH 02/30] refactor(utility): centralized validation in create and apply --- utility/types/error.go | 8 ++- utility/unit_of_work/block.go | 7 ++- utility/unit_of_work/block_test.go | 7 +-- utility/unit_of_work/module.go | 88 +----------------------------- utility/unit_of_work/uow_leader.go | 7 +-- 5 files changed, 19 insertions(+), 98 deletions(-) diff --git a/utility/types/error.go b/utility/types/error.go index 122268625..39aa11da8 100644 --- a/utility/types/error.go +++ b/utility/types/error.go @@ -40,7 +40,7 @@ func NewError(code Code, msg string) Error { } } -// NextCode: 134 +// NextCode: 135 type Code float64 // CONSIDERATION: Should these be a proto enum or a golang iota? //nolint:gosec // G101 - Not hard-coded credentials @@ -115,6 +115,7 @@ const ( CodeInvalidServiceURLError Code = 70 CodeNotExistsError Code = 71 CodeGetMissedBlocksError Code = 72 + CodeGetPrevBlockByzantineValidators Code = 134 CodeEmptyHashError Code = 73 CodeInvalidBlockHeightError Code = 74 CodeUnequalPublicKeysError Code = 75 @@ -185,6 +186,7 @@ const ( UnequalVoteTypesError = "the vote types are not equal" UnequalPublicKeysError = "the two public keys are not equal" GetMissedBlocksError = "an error occurred getting the missed blocks field" + GetPrevBlockByzantineValidators = "an error occurred getting the previous block's byzantine validators" DecodeMessageError = "unable to decode the message" NotExistsError = "the actor does not exist in the state" InvalidServiceURLError = "the service url is not valid" @@ -368,6 +370,10 @@ func ErrGetMissedBlocks(err error) Error { return NewError(CodeGetMissedBlocksError, fmt.Sprintf("%s: %s", GetMissedBlocksError, err.Error())) } +func ErrGetPrevBlockByzantineValidators(err error) Error { + return NewError(CodeGetPrevBlockByzantineValidators, fmt.Sprintf("%s: %s", GetPrevBlockByzantineValidators, err.Error())) +} + func ErrGetStakedTokens(err error) Error { return NewError(CodeGetStakedAmountError, GetStakedAmountsError) } diff --git a/utility/unit_of_work/block.go b/utility/unit_of_work/block.go index 5d5893c87..b41da9e5a 100644 --- a/utility/unit_of_work/block.go +++ b/utility/unit_of_work/block.go @@ -12,7 +12,12 @@ import ( typesUtil "github.com/pokt-network/pocket/utility/types" ) -func (u *baseUtilityUnitOfWork) beginBlock(previousBlockByzantineValidators [][]byte) typesUtil.Error { +func (u *baseUtilityUnitOfWork) beginBlock() typesUtil.Error { + previousBlockByzantineValidators, err := u.prevBlockByzantineValidators() + if err != nil { + return typesUtil.ErrGetPrevBlockByzantineValidators(err) + } + if err := u.handleByzantineValidators(previousBlockByzantineValidators); err != nil { return err } diff --git a/utility/unit_of_work/block_test.go b/utility/unit_of_work/block_test.go index 9ccfa1044..b416feaa1 100644 --- a/utility/unit_of_work/block_test.go +++ b/utility/unit_of_work/block_test.go @@ -75,7 +75,6 @@ func TestUtilityUnitOfWork_ApplyBlock(t *testing.T) { proposerBalanceDifference := big.NewInt(0).Sub(proposerAfterBalance, proposerBeforeBalance) require.Equal(t, expectedProposerBalanceDifference, proposerBalanceDifference, "unexpected before / after balance difference") - } func TestUtilityUnitOfWork_BeginBlock(t *testing.T) { @@ -90,7 +89,7 @@ func TestUtilityUnitOfWork_BeginBlock(t *testing.T) { addrBz, er := hex.DecodeString(proposer.GetAddress()) require.NoError(t, er) - er = uow.SetProposalBlock("", addrBz, [][]byte{txBz}) + er = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) require.NoError(t, er) _, _, er = uow.ApplyBlock() @@ -101,7 +100,6 @@ func TestUtilityUnitOfWork_BeginBlock(t *testing.T) { // missed, err := ctx.getValidatorMissedBlocks(byzantine.Address) // require.NoError(t, err) // require.Equal(t, missed, 1) - } func TestUtilityUnitOfWork_EndBlock(t *testing.T) { @@ -119,7 +117,7 @@ func TestUtilityUnitOfWork_EndBlock(t *testing.T) { proposerBeforeBalance, err := uow.getAccountAmount(addrBz) require.NoError(t, err) - er = uow.SetProposalBlock("", addrBz, [][]byte{txBz}) + er = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) require.NoError(t, er) _, _, er = uow.ApplyBlock() @@ -140,5 +138,4 @@ func TestUtilityUnitOfWork_EndBlock(t *testing.T) { proposerBalanceDifference := big.NewInt(0).Sub(proposerAfterBalance, proposerBeforeBalance) require.Equal(t, expectedProposerBalanceDifference, proposerBalanceDifference) - } diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 7c9bb30e0..0a2d2c687 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -44,95 +44,13 @@ func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAdd return nil } -// CreateAndApplyProposalBlock implements the exposed functionality of the shared UtilityUnitOfWork interface. -func (u *baseUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, maxTransactionBytes int) (stateHash string, txs [][]byte, err error) { - prevBlockByzantineVals, err := u.prevBlockByzantineValidators() - if err != nil { - return "", nil, err - } - - // begin block lifecycle phase - if err := u.beginBlock(prevBlockByzantineVals); err != nil { - return "", nil, err - } - txs = make([][]byte, 0) - txsTotalBz := 0 - txIdx := 0 - - mempool := u.GetBus().GetUtilityModule().GetMempool() - for !mempool.IsEmpty() { - // NB: In order for transactions to have entered the mempool, `HandleTransaction` must have - // been called which handles basic checks & validation. - txBz, err := mempool.PopTx() - if err != nil { - return "", nil, err - } - - tx, err := coreTypes.TxFromBytes(txBz) - if err != nil { - return "", nil, err - } - - txBzSize := len(txBz) - txsTotalBz += txBzSize - - // Exceeding maximum transaction bytes to be added in this block - if txsTotalBz >= maxTransactionBytes { - // Add back popped tx to be applied in a future block - if err := mempool.AddTx(txBz); err != nil { - return "", nil, err - } - break // we've reached our max - } - - txResult, err := u.hydrateTxResult(tx, txIdx) - if err != nil { - u.logger.Err(err).Msg("Error in ApplyTransaction") - // TODO(#327): Properly implement 'unhappy path' for save points - if err := u.revertLastSavePoint(); err != nil { - return "", nil, err - } - txsTotalBz -= txBzSize - continue - } - - // Index the transaction - if err := u.persistenceRWContext.IndexTransaction(txResult); err != nil { - u.logger.Fatal().Err(err).Msgf("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now: %v\n", err) - } - - txs = append(txs, txBz) - txIdx++ - } - - if err := u.endBlock(proposer); err != nil { - return "", nil, err - } - - // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) - // Compute & return the new state hash - stateHash, err = u.persistenceRWContext.ComputeStateHash() - if err != nil { - u.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") - } - u.logger.Info().Str("state_hash", stateHash).Msgf("CreateAndApplyProposalBlock finished successfully") - - return stateHash, txs, err -} - // CLEANUP: code re-use ApplyBlock() for CreateAndApplyBlock() -func (u *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, err error) { +func (u *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { if !u.isProposalBlockSet() { return "", nil, utilTypes.ErrProposalBlockNotSet() } - - lastByzantineValidators, err := u.prevBlockByzantineValidators() - if err != nil { - return "", nil, err - } - // begin block lifecycle phase - if err := u.beginBlock(lastByzantineValidators); err != nil { + if err := u.beginBlock(); err != nil { return "", nil, err } @@ -183,7 +101,7 @@ func (u *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, er } // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // return the app hash (consensus module will get the validator set directly) - stateHash, err = u.persistenceRWContext.ComputeStateHash() + stateHash, err := u.persistenceRWContext.ComputeStateHash() if err != nil { u.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") return "", nil, utilTypes.ErrAppHash(err) diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index f2dcae33e..05253216a 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -27,13 +27,8 @@ func NewLeaderUOW(height int64, readContext modules.PersistenceReadContext, rwPe } func (uow *leaderUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) { - prevBlockByzantineVals, err := uow.prevBlockByzantineValidators() - if err != nil { - return "", nil, err - } - // begin block lifecycle phase - if err := uow.beginBlock(prevBlockByzantineVals); err != nil { + if err := uow.beginBlock(); err != nil { return "", nil, err } txs = make([][]byte, 0) From 7d3bf1903ace5564a758d6a9ced7652bcee6709e Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:00:57 +0100 Subject: [PATCH 03/30] fix(utility): fix isProposalBlockSet --- utility/unit_of_work/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 0a2d2c687..b08606cbb 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -140,5 +140,5 @@ func (uow *baseUtilityUnitOfWork) Release() error { } func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { - return uow.proposalStateHash != "" && uow.proposalProposerAddr != nil && uow.proposalBlockTxs != nil + return uow.proposalStateHash != "" && uow.proposalProposerAddr != nil } From 096470eaa965fc5310b00078db55d8d786f1a9c4 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:02:58 +0100 Subject: [PATCH 04/30] refactor(shared): CreateAndApplyProposalBlock & ApplyBlock --- shared/docs/PROTOCOL_STATE_HASH.md | 8 ++++---- shared/modules/utility_module.go | 8 +++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/shared/docs/PROTOCOL_STATE_HASH.md b/shared/docs/PROTOCOL_STATE_HASH.md index 1cf8afaa2..f2c5ca4b1 100644 --- a/shared/docs/PROTOCOL_STATE_HASH.md +++ b/shared/docs/PROTOCOL_STATE_HASH.md @@ -94,7 +94,7 @@ sequenceDiagram When applying the block during the `NEWROUND` message shown above, the majority of the flow is similar between the _leader_ and the _replica_ with one of the major differences being a call to the `Utility` module as seen below. - `ApplyBlock` - Uses the existing set of transactions to validate & propose -- `CreateAndApplyProposalBlock` - Reaps the mempool for a new set of transaction to validate and propose +- `CreateProposalBlock` - Reaps the mempool for a new set of transaction to validate and propose ```mermaid graph TD @@ -115,14 +115,14 @@ graph TD I[Is prepareQC.view > lockedQC.view] --> |"No
(lockedQC.block)"| Z I[Is prepareQC.view > lockedQC.view] --> |"Yes
(prepareQC.block)"| Z - H[CreateAndApplyProposalBlock] + H[CreateProposalBlock] Z[ApplyBlock] ``` As either the _leader_ or _replica_, the following steps are followed to apply the proposal transactions in the block. 1. Update the `UtilityUnitOfWork` with the proposed block -2. Call either `ApplyBlock` or `CreateAndApplyProposalBlock` based on the flow above +2. Call either `ApplyBlock` or `CreateProposalBlock` based on the flow above ```mermaid sequenceDiagram @@ -135,7 +135,7 @@ sequenceDiagram U->>-C: err_code %% Apply the block to the local proposal state - C->>+U: ApplyBlock / CreateAndApplyProposalBlock + C->>+U: ApplyBlock / CreateProposalBlock U->>-C: err_code ``` diff --git a/shared/modules/utility_module.go b/shared/modules/utility_module.go index 7edc6f48d..3d2923155 100644 --- a/shared/modules/utility_module.go +++ b/shared/modules/utility_module.go @@ -42,7 +42,7 @@ type UnstakingActor interface { // CONSIDERATION: Consider removing `Utility` from `UtilityUnitOfWork` altogether -// TECHDEBT(@deblasis): `CreateAndApplyProposalBlock` and `ApplyBlock` should be be refactored into a +// TECHDEBT(@deblasis): `CreateProposalBlock` and `ApplyBlock` should be be refactored into a // `GetProposalBlock` and `ApplyProposalBlock` functions // UtilityUnitOfWork is a unit of work (https://martinfowler.com/eaaCatalog/unitOfWork.html) that allows for atomicity and commit/rollback functionality @@ -53,7 +53,6 @@ type UtilityUnitOfWork interface { // It does not apply, validate or commit the changes. // For example, it can be use during state sync to set a proposed state transition before validation. // TODO: Investigate a way to potentially simplify the interface by removing this function. - // TODO: @deblasis: there's still some mix and match between blockHash and stateHash SetProposalBlock(blockHash string, proposerAddr []byte, txs [][]byte) error // ApplyBlock applies the context's in-memory proposed state (i.e. the txs in this context). @@ -71,10 +70,9 @@ type UtilityUnitOfWork interface { type LeaderUtilityUnitOfWork interface { UtilityUnitOfWork - // CreateAndApplyProposalBlock reaps the mempool for txs to be proposed in a new block, and + // CreateProposalBlock reaps the mempool for txs to be proposed in a new block, and // applies them to this context after validation. - // TODO: #508 new signature - CreateAndApplyProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) + CreateProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) } type ReplicaUtilityUnitOfWork interface { From d6335e4960cf15097d40c0c46cf2c27ab67611d9 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:03:17 +0100 Subject: [PATCH 05/30] refactor(consensus): CreateAndApplyProposalBlock & ApplyBlock --- consensus/e2e_tests/utils_test.go | 2 +- consensus/hotstuff_leader.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index af23325eb..25034b028 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -459,7 +459,7 @@ func baseLeaderUtilityUnitOfWorkMock(t *testing.T, genesisState *genesis.Genesis rwContextMock.EXPECT().Release().AnyTimes() utilityLeaderUnitOfWorkMock.EXPECT(). - CreateAndApplyProposalBlock(gomock.Any(), maxTxBytes). + CreateProposalBlock(gomock.Any(), maxTxBytes). Return(stateHash, make([][]byte, 0), nil). AnyTimes() utilityLeaderUnitOfWorkMock.EXPECT(). diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index e5b0491f1..eb9374967 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -399,7 +399,7 @@ func (m *consensusModule) prepareBlock(qc *typesCons.QuorumCertificate) (*coreTy } // Reap the mempool for transactions to be applied in this block - stateHash, txs, err := leaderUOW.CreateAndApplyProposalBlock(m.privateKey.Address(), maxTxBytes) + stateHash, txs, err := leaderUOW.CreateProposalBlock(m.privateKey.Address(), maxTxBytes) if err != nil { return nil, err From 1ab4d5f1f18c5bc7f5d7df93130ce51edff94496 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:03:35 +0100 Subject: [PATCH 06/30] refactor(utility): CreateAndApplyProposalBlock & ApplyBlock --- utility/unit_of_work/module.go | 80 ++++++++++++++++-------------- utility/unit_of_work/uow_leader.go | 52 +++++++++++-------- 2 files changed, 73 insertions(+), 59 deletions(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index b08606cbb..99e774608 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -2,6 +2,7 @@ package unit_of_work import ( coreTypes "github.com/pokt-network/pocket/shared/core/types" + "github.com/pokt-network/pocket/shared/mempool" "github.com/pokt-network/pocket/shared/modules" "github.com/pokt-network/pocket/shared/modules/base_modules" utilTypes "github.com/pokt-network/pocket/utility/types" @@ -55,44 +56,8 @@ func (u *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { } mempool := u.GetBus().GetUtilityModule().GetMempool() - - // deliver txs lifecycle phase - for index, txProtoBytes := range u.proposalBlockTxs { - tx, err := coreTypes.TxFromBytes(txProtoBytes) - if err != nil { - return "", nil, err - } - if err := tx.ValidateBasic(); err != nil { - return "", nil, err - } - // TODO(#346): Currently, the pattern is allowing nil err with an error transaction... - // Should we terminate applyBlock immediately if there's an invalid transaction? - // Or wait until the entire lifecycle is over to evaluate an 'invalid' block - - // Validate and apply the transaction to the Postgres database - txResult, err := u.hydrateTxResult(tx, index) - if err != nil { - return "", nil, err - } - - txHash, err := tx.Hash() - if err != nil { - return "", nil, err - } - - // TODO: Need to properly add transactions back on rollbacks - if mempool.Contains(txHash) { - if err := mempool.RemoveTx(txProtoBytes); err != nil { - return "", nil, err - } - u.logger.Info().Str("tx_hash", txHash).Msg("Applying tx that WAS in the local mempool") - } else { - u.logger.Info().Str("tx_hash", txHash).Msg("Applying tx that WAS NOT in the local mempool") - } - - if err := u.persistenceRWContext.IndexTransaction(txResult); err != nil { - u.logger.Fatal().Err(err).Msgf("TODO(#327): We can apply the transaction but not index it. Crash the process for now: %v\n", err) - } + if err := u.processTransactionsFromProposalBlock(mempool, u.proposalBlockTxs); err != nil { + return "", nil, err } // end block lifecycle phase @@ -142,3 +107,42 @@ func (uow *baseUtilityUnitOfWork) Release() error { func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { return uow.proposalStateHash != "" && uow.proposalProposerAddr != nil } + +// processTransactionsFromProposalBlock processes the transactions from the proposal block. +// It also removes the transactions from the mempool if they are already present. +func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(mempool mempool.TXMempool, txsBytes [][]byte) (err error) { + for index, txProtoBytes := range txsBytes { + tx, err := coreTypes.TxFromBytes(txProtoBytes) + if err != nil { + return err + } + if err := tx.ValidateBasic(); err != nil { + return err + } + + txResult, err := uow.hydrateTxResult(tx, index) + if err != nil { + return err + } + + txHash, err := tx.Hash() + if err != nil { + return err + } + + if mempool.Contains(txHash) { + if err := mempool.RemoveTx(txProtoBytes); err != nil { + return err + } + uow.logger.Info().Str("tx_hash", txHash).Msg("Applying tx that WAS in the local mempool") + } else { + uow.logger.Info().Str("tx_hash", txHash).Msg("Applying tx that WAS NOT in the local mempool") + } + + // TODO(#564): make sure that indexing is reversible in case of a rollback + if err := uow.persistenceRWContext.IndexTransaction(txResult); err != nil { + uow.logger.Fatal().Err(err).Msgf("TODO(#327): We can apply the transaction but not index it. Crash the process for now: %v\n", err) + } + } + return nil +} diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 05253216a..adeb6f7c2 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -3,6 +3,7 @@ package unit_of_work import ( "github.com/pokt-network/pocket/logger" coreTypes "github.com/pokt-network/pocket/shared/core/types" + "github.com/pokt-network/pocket/shared/mempool" "github.com/pokt-network/pocket/shared/modules" ) @@ -26,27 +27,48 @@ func NewLeaderUOW(height int64, readContext modules.PersistenceReadContext, rwPe } } -func (uow *leaderUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) { +func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) { // begin block lifecycle phase if err := uow.beginBlock(); err != nil { return "", nil, err } + + mempool := uow.GetBus().GetUtilityModule().GetMempool() + if txs, err = uow.reapMempool(mempool, maxTxBytes); err != nil { + return "", nil, err + } + + if err := uow.endBlock(proposer); err != nil { + return "", nil, err + } + + // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) + // Compute & return the new state hash + stateHash, err = uow.persistenceRWContext.ComputeStateHash() + if err != nil { + uow.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + } + uow.logger.Info().Str("state_hash", stateHash).Msgf("CreateProposalBlock finished successfully") + + return stateHash, txs, err +} + +// reapMempool reaps transactions from the mempool up to the maximum transaction bytes allowed in a block. +func (uow *leaderUtilityUnitOfWork) reapMempool(mempool mempool.TXMempool, maxTxBytes uint64) (txs [][]byte, err error) { txs = make([][]byte, 0) txsTotalBz := uint64(0) txIdx := 0 - - mempool := uow.GetBus().GetUtilityModule().GetMempool() for !mempool.IsEmpty() { // NB: In order for transactions to have entered the mempool, `HandleTransaction` must have // been called which handles basic checks & validation. txBz, err := mempool.PopTx() if err != nil { - return "", nil, err + return nil, err } tx, err := coreTypes.TxFromBytes(txBz) if err != nil { - return "", nil, err + return nil, err } txBzSize := uint64(len(txBz)) @@ -56,7 +78,7 @@ func (uow *leaderUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, if txsTotalBz >= maxTxBytes { // Add back popped tx to be applied in a future block if err := mempool.AddTx(txBz); err != nil { - return "", nil, err + return nil, err } break // we've reached our max } @@ -66,12 +88,13 @@ func (uow *leaderUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, uow.logger.Err(err).Msg("Error in ApplyTransaction") // TODO(#327): Properly implement 'unhappy path' for save points if err := uow.revertLastSavePoint(); err != nil { - return "", nil, err + return nil, err } txsTotalBz -= txBzSize continue } + // TODO(#564): make sure that indexing is reversible in case of a rollback // Index the transaction if err := uow.persistenceRWContext.IndexTransaction(txResult); err != nil { uow.logger.Fatal().Err(err).Msgf("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now: %v\n", err) @@ -80,18 +103,5 @@ func (uow *leaderUtilityUnitOfWork) CreateAndApplyProposalBlock(proposer []byte, txs = append(txs, txBz) txIdx++ } - - if err := uow.endBlock(proposer); err != nil { - return "", nil, err - } - - // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) - // Compute & return the new state hash - stateHash, err = uow.persistenceRWContext.ComputeStateHash() - if err != nil { - uow.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") - } - uow.logger.Info().Str("state_hash", stateHash).Msgf("CreateAndApplyProposalBlock finished successfully") - - return stateHash, txs, err + return } From 54af6c9e32db7b68e885d1e0205f352ed9ed8c48 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:19:43 +0100 Subject: [PATCH 07/30] refactor(utility): instrumenting with logs --- utility/unit_of_work/block.go | 79 +++++++++++++++++------------- utility/unit_of_work/module.go | 28 +++++++---- utility/unit_of_work/uow_leader.go | 16 +++++- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/utility/unit_of_work/block.go b/utility/unit_of_work/block.go index b41da9e5a..d43c2202c 100644 --- a/utility/unit_of_work/block.go +++ b/utility/unit_of_work/block.go @@ -12,32 +12,45 @@ import ( typesUtil "github.com/pokt-network/pocket/utility/types" ) -func (u *baseUtilityUnitOfWork) beginBlock() typesUtil.Error { - previousBlockByzantineValidators, err := u.prevBlockByzantineValidators() +func (uow *baseUtilityUnitOfWork) beginBlock() typesUtil.Error { + log := uow.logger.With().Fields(map[string]interface{}{ + "source": "beginBlock", + }).Logger() + + log.Debug().Bool("TODO", true).Msg("determining prevBlockByzantineValidators") + previousBlockByzantineValidators, err := uow.prevBlockByzantineValidators() if err != nil { return typesUtil.ErrGetPrevBlockByzantineValidators(err) } - if err := u.handleByzantineValidators(previousBlockByzantineValidators); err != nil { + log.Info().Msg("handling byzantine validators") + if err := uow.handleByzantineValidators(previousBlockByzantineValidators); err != nil { return err } // INCOMPLETE: Identify what else needs to be done in the begin block lifecycle phase return nil } -func (u *baseUtilityUnitOfWork) endBlock(proposer []byte) typesUtil.Error { +func (uow *baseUtilityUnitOfWork) endBlock(proposer []byte) typesUtil.Error { + log := uow.logger.With().Fields(map[string]interface{}{ + "source": "endBlock", + }).Logger() + + log.Info().Msg("handling proposer rewards") // reward the block proposer - if err := u.handleProposerRewards(proposer); err != nil { + if err := uow.handleProposerRewards(proposer); err != nil { return err } + log.Info().Msg("handling unstaking actors") // unstake actors that have been 'unstaking' for the UnstakingBlocks - if err := u.unbondUnstakingActors(); err != nil { + if err := uow.unbondUnstakingActors(); err != nil { return err } + log.Info().Msg("handling unstaking paused actors") // begin unstaking the actors who have been paused for MaxPauseBlocks - if err := u.beginUnstakingMaxPausedActors(); err != nil { + if err := uow.beginUnstakingMaxPausedActors(); err != nil { return err } @@ -45,20 +58,20 @@ func (u *baseUtilityUnitOfWork) endBlock(proposer []byte) typesUtil.Error { return nil } -func (u *baseUtilityUnitOfWork) handleProposerRewards(proposer []byte) typesUtil.Error { +func (uow *baseUtilityUnitOfWork) handleProposerRewards(proposer []byte) typesUtil.Error { feePoolName := coreTypes.Pools_POOLS_FEE_COLLECTOR.FriendlyName() - feesAndRewardsCollected, err := u.getPoolAmount(feePoolName) + feesAndRewardsCollected, err := uow.getPoolAmount(feePoolName) if err != nil { return err } // Nullify the rewards pool - if err := u.setPoolAmount(feePoolName, big.NewInt(0)); err != nil { + if err := uow.setPoolAmount(feePoolName, big.NewInt(0)); err != nil { return err } // - proposerCutPercentage, err := u.getProposerPercentageOfFees() + proposerCutPercentage, err := uow.getProposerPercentageOfFees() if err != nil { return err } @@ -73,16 +86,16 @@ func (u *baseUtilityUnitOfWork) handleProposerRewards(proposer []byte) typesUtil amountToProposerFloat.Quo(amountToProposerFloat, big.NewFloat(100)) amountToProposer, _ := amountToProposerFloat.Int(nil) amountToDAO := feesAndRewardsCollected.Sub(feesAndRewardsCollected, amountToProposer) - if err := u.addAccountAmount(proposer, amountToProposer); err != nil { + if err := uow.addAccountAmount(proposer, amountToProposer); err != nil { return err } - if err := u.addPoolAmount(coreTypes.Pools_POOLS_DAO.FriendlyName(), amountToDAO); err != nil { + if err := uow.addPoolAmount(coreTypes.Pools_POOLS_DAO.FriendlyName(), amountToDAO); err != nil { return err } return nil } -func (u *baseUtilityUnitOfWork) unbondUnstakingActors() (err typesUtil.Error) { +func (uow *baseUtilityUnitOfWork) unbondUnstakingActors() (err typesUtil.Error) { for actorTypeNum := range coreTypes.ActorType_name { if actorTypeNum == 0 { // ACTOR_TYPE_UNSPECIFIED continue @@ -95,16 +108,16 @@ func (u *baseUtilityUnitOfWork) unbondUnstakingActors() (err typesUtil.Error) { var er error switch actorType { case coreTypes.ActorType_ACTOR_TYPE_APP: - readyToUnbond, er = u.persistenceReadContext.GetAppsReadyToUnstake(u.height, int32(coreTypes.StakeStatus_Unstaking)) + readyToUnbond, er = uow.persistenceReadContext.GetAppsReadyToUnstake(uow.height, int32(coreTypes.StakeStatus_Unstaking)) poolName = coreTypes.Pools_POOLS_APP_STAKE.FriendlyName() case coreTypes.ActorType_ACTOR_TYPE_FISH: - readyToUnbond, er = u.persistenceReadContext.GetFishermenReadyToUnstake(u.height, int32(coreTypes.StakeStatus_Unstaking)) + readyToUnbond, er = uow.persistenceReadContext.GetFishermenReadyToUnstake(uow.height, int32(coreTypes.StakeStatus_Unstaking)) poolName = coreTypes.Pools_POOLS_FISHERMAN_STAKE.FriendlyName() case coreTypes.ActorType_ACTOR_TYPE_SERVICER: - readyToUnbond, er = u.persistenceReadContext.GetServicersReadyToUnstake(u.height, int32(coreTypes.StakeStatus_Unstaking)) + readyToUnbond, er = uow.persistenceReadContext.GetServicersReadyToUnstake(uow.height, int32(coreTypes.StakeStatus_Unstaking)) poolName = coreTypes.Pools_POOLS_SERVICER_STAKE.FriendlyName() case coreTypes.ActorType_ACTOR_TYPE_VAL: - readyToUnbond, er = u.persistenceReadContext.GetValidatorsReadyToUnstake(u.height, int32(coreTypes.StakeStatus_Unstaking)) + readyToUnbond, er = uow.persistenceReadContext.GetValidatorsReadyToUnstake(uow.height, int32(coreTypes.StakeStatus_Unstaking)) poolName = coreTypes.Pools_POOLS_VALIDATOR_STAKE.FriendlyName() case coreTypes.ActorType_ACTOR_TYPE_UNSPECIFIED: continue @@ -126,10 +139,10 @@ func (u *baseUtilityUnitOfWork) unbondUnstakingActors() (err typesUtil.Error) { return typesUtil.ErrHexDecodeFromString(err) } - if err := u.subPoolAmount(poolName, stakeAmount); err != nil { + if err := uow.subPoolAmount(poolName, stakeAmount); err != nil { return err } - if err := u.addAccountAmount(outputAddrBz, stakeAmount); err != nil { + if err := uow.addAccountAmount(outputAddrBz, stakeAmount); err != nil { return err } } @@ -138,7 +151,7 @@ func (u *baseUtilityUnitOfWork) unbondUnstakingActors() (err typesUtil.Error) { return nil } -func (u *baseUtilityUnitOfWork) beginUnstakingMaxPausedActors() (err typesUtil.Error) { +func (uow *baseUtilityUnitOfWork) beginUnstakingMaxPausedActors() (err typesUtil.Error) { for actorTypeNum := range coreTypes.ActorType_name { if actorTypeNum == 0 { // ACTOR_TYPE_UNSPECIFIED continue @@ -148,23 +161,23 @@ func (u *baseUtilityUnitOfWork) beginUnstakingMaxPausedActors() (err typesUtil.E if actorType == coreTypes.ActorType_ACTOR_TYPE_UNSPECIFIED { continue } - maxPausedBlocks, err := u.getMaxAllowedPausedBlocks(actorType) + maxPausedBlocks, err := uow.getMaxAllowedPausedBlocks(actorType) if err != nil { return err } - maxPauseHeight := u.height - int64(maxPausedBlocks) + maxPauseHeight := uow.height - int64(maxPausedBlocks) if maxPauseHeight < 0 { // genesis edge case maxPauseHeight = 0 } - if err := u.beginUnstakingActorsPausedBefore(maxPauseHeight, actorType); err != nil { + if err := uow.beginUnstakingActorsPausedBefore(maxPauseHeight, actorType); err != nil { return err } } return nil } -func (u *baseUtilityUnitOfWork) beginUnstakingActorsPausedBefore(pausedBeforeHeight int64, actorType coreTypes.ActorType) (err typesUtil.Error) { - unbondingHeight, err := u.getUnbondingHeight(actorType) +func (uow *baseUtilityUnitOfWork) beginUnstakingActorsPausedBefore(pausedBeforeHeight int64, actorType coreTypes.ActorType) (err typesUtil.Error) { + unbondingHeight, err := uow.getUnbondingHeight(actorType) if err != nil { return err } @@ -172,13 +185,13 @@ func (u *baseUtilityUnitOfWork) beginUnstakingActorsPausedBefore(pausedBeforeHei var er error switch actorType { case coreTypes.ActorType_ACTOR_TYPE_APP: - er = u.persistenceRWContext.SetAppStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) + er = uow.persistenceRWContext.SetAppStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) case coreTypes.ActorType_ACTOR_TYPE_FISH: - er = u.persistenceRWContext.SetFishermanStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) + er = uow.persistenceRWContext.SetFishermanStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) case coreTypes.ActorType_ACTOR_TYPE_SERVICER: - er = u.persistenceRWContext.SetServicerStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) + er = uow.persistenceRWContext.SetServicerStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) case coreTypes.ActorType_ACTOR_TYPE_VAL: - er = u.persistenceRWContext.SetValidatorsStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) + er = uow.persistenceRWContext.SetValidatorsStatusAndUnstakingHeightIfPausedBefore(pausedBeforeHeight, unbondingHeight, int32(coreTypes.StakeStatus_Unstaking)) } if er != nil { return typesUtil.ErrSetStatusPausedBefore(er, pausedBeforeHeight) @@ -187,12 +200,12 @@ func (u *baseUtilityUnitOfWork) beginUnstakingActorsPausedBefore(pausedBeforeHei } // TODO: Need to design & document this business logic. -func (u *baseUtilityUnitOfWork) prevBlockByzantineValidators() ([][]byte, error) { +func (uow *baseUtilityUnitOfWork) prevBlockByzantineValidators() ([][]byte, error) { return nil, nil } // TODO: This has not been tested or investigated in detail -func (u *baseUtilityUnitOfWork) revertLastSavePoint() typesUtil.Error { +func (uow *baseUtilityUnitOfWork) revertLastSavePoint() typesUtil.Error { // TODO(@deblasis): Implement this // if len(u.savePointsSet) == 0 { // return typesUtil.ErrEmptySavePoints() @@ -208,7 +221,7 @@ func (u *baseUtilityUnitOfWork) revertLastSavePoint() typesUtil.Error { } //nolint:unused // TODO: This has not been tested or investigated in detail -func (u *baseUtilityUnitOfWork) newSavePoint(txHashBz []byte) typesUtil.Error { +func (uow *baseUtilityUnitOfWork) newSavePoint(txHashBz []byte) typesUtil.Error { // TODO(@deblasis): Implement this // if err := u.store.NewSavePoint(txHashBz); err != nil { // return typesUtil.ErrNewSavePoint(err) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 99e774608..9f8b0670c 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -46,32 +46,42 @@ func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAdd } // CLEANUP: code re-use ApplyBlock() for CreateAndApplyBlock() -func (u *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { - if !u.isProposalBlockSet() { +func (uow *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { + log := uow.logger.With().Fields(map[string]interface{}{ + "source": "ApplyBlock", + }).Logger() + + log.Debug().Msg("checking if proposal block has been set") + if !uow.isProposalBlockSet() { return "", nil, utilTypes.ErrProposalBlockNotSet() } + // begin block lifecycle phase - if err := u.beginBlock(); err != nil { + log.Debug().Msg("calling beginBlock") + if err := uow.beginBlock(); err != nil { return "", nil, err } - mempool := u.GetBus().GetUtilityModule().GetMempool() - if err := u.processTransactionsFromProposalBlock(mempool, u.proposalBlockTxs); err != nil { + log.Debug().Msg("processing transactions from proposal block") + mempool := uow.GetBus().GetUtilityModule().GetMempool() + if err := uow.processTransactionsFromProposalBlock(mempool, uow.proposalBlockTxs); err != nil { return "", nil, err } // end block lifecycle phase - if err := u.endBlock(u.proposalProposerAddr); err != nil { + log.Debug().Msg("calling endBlock") + if err := uow.endBlock(uow.proposalProposerAddr); err != nil { return "", nil, err } // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // return the app hash (consensus module will get the validator set directly) - stateHash, err := u.persistenceRWContext.ComputeStateHash() + log.Debug().Msg("computing state hash") + stateHash, err := uow.persistenceRWContext.ComputeStateHash() if err != nil { - u.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") return "", nil, utilTypes.ErrAppHash(err) } - u.logger.Info().Str("state_hash", stateHash).Msgf("ApplyBlock succeeded!") + log.Info().Str("state_hash", stateHash).Msgf("ApplyBlock succeeded!") // return the app hash; consensus module will get the validator set directly return stateHash, nil, nil diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index adeb6f7c2..99975df45 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -1,6 +1,8 @@ package unit_of_work import ( + "encoding/hex" + "github.com/pokt-network/pocket/logger" coreTypes "github.com/pokt-network/pocket/shared/core/types" "github.com/pokt-network/pocket/shared/mempool" @@ -28,27 +30,37 @@ func NewLeaderUOW(height int64, readContext modules.PersistenceReadContext, rwPe } func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) { + log := uow.logger.With().Fields(map[string]interface{}{ + "proposer": hex.EncodeToString(proposer), + "maxTxBytes": maxTxBytes, + "source": "CreateProposalBlock", + }).Logger() + log.Debug().Msg("calling beginBlock") // begin block lifecycle phase if err := uow.beginBlock(); err != nil { return "", nil, err } + log.Debug().Msg("reaping the mempool") mempool := uow.GetBus().GetUtilityModule().GetMempool() if txs, err = uow.reapMempool(mempool, maxTxBytes); err != nil { return "", nil, err } + // end block lifecycle phase + log.Debug().Msg("calling endBlock") if err := uow.endBlock(proposer); err != nil { return "", nil, err } + log.Debug().Msg("computing state hash") // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // Compute & return the new state hash stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { - uow.logger.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") } - uow.logger.Info().Str("state_hash", stateHash).Msgf("CreateProposalBlock finished successfully") + log.Info().Str("state_hash", stateHash).Msgf("Finished successfully") return stateHash, txs, err } From 88388f849e64cb54c7a12065b1ab5f9a97880b20 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:26:58 +0100 Subject: [PATCH 08/30] fix(utility): logging proposer in endBlock --- utility/unit_of_work/block.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utility/unit_of_work/block.go b/utility/unit_of_work/block.go index d43c2202c..442499fa8 100644 --- a/utility/unit_of_work/block.go +++ b/utility/unit_of_work/block.go @@ -33,7 +33,8 @@ func (uow *baseUtilityUnitOfWork) beginBlock() typesUtil.Error { func (uow *baseUtilityUnitOfWork) endBlock(proposer []byte) typesUtil.Error { log := uow.logger.With().Fields(map[string]interface{}{ - "source": "endBlock", + "proposer": hex.EncodeToString(proposer), + "source": "endBlock", }).Logger() log.Info().Msg("handling proposer rewards") From f6cf1482924dea49f46b784e8158ce422ff4f181 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:37:04 +0100 Subject: [PATCH 09/30] docs(consensus): CHANGELOG --- consensus/doc/CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index 19c1fb897..bc1d76bb1 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,12 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.43] - 2023-04-04 + +- Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` + ## [0.0.0.42] - 2023-04-03 - Add `fsm_handler.go` to handle FSM transition events in consensus module - Update State Machine mock in `utils_test.go` -- Update state_sync module with additional function definitions - +- Update state_sync module with additional function definitions + ## [0.0.0.41] - 2023-03-30 - Improve & simplify `utilityUnitOfWork` management From 719c4362a7e4360967823fe4c8b65203be5d0917 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:37:56 +0100 Subject: [PATCH 10/30] docs(shared): CHANGELOG --- shared/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shared/CHANGELOG.md b/shared/CHANGELOG.md index 87852bbae..369c8de58 100644 --- a/shared/CHANGELOG.md +++ b/shared/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.47] - 2023-04-04 + +- Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` + ## [0.0.0.46] - 2023-04-03 - Add `ConsensusStateSync` interface. It defines exported state sync functions in consensus module From af89d70176b95bd94fd6ea199263fb3096069db1 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:40:26 +0100 Subject: [PATCH 11/30] docs(utility): CHANGELOG --- utility/doc/CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/utility/doc/CHANGELOG.md b/utility/doc/CHANGELOG.md index 8ae86aa53..d216663da 100644 --- a/utility/doc/CHANGELOG.md +++ b/utility/doc/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.34] - 2023-04-04 + +- Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` +- Added `GetPrevBlockByzantineValidators` and `ProposalBlockNotSet` errors +- Instrumented `CreateProposalBlock` and `ApplyBlock` with log statements +- Refactored functions for block creation and application to be more readable/modular +- Added TODOs for future refactoring +- Renamed `u` to `uow` for consistency + ## [0.0.0.33] - 2023-03-30 - Improved logging throughout the module From 33e19c15bb707879e58a50144452c4d2fc091f33 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 4 Apr 2023 23:56:34 +0100 Subject: [PATCH 12/30] chore(utility): lint --- utility/unit_of_work/module.go | 14 +++++++------- utility/unit_of_work/uow_leader.go | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 9f8b0670c..975024613 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -46,7 +46,7 @@ func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAdd } // CLEANUP: code re-use ApplyBlock() for CreateAndApplyBlock() -func (uow *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { +func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, err error) { log := uow.logger.With().Fields(map[string]interface{}{ "source": "ApplyBlock", }).Logger() @@ -63,8 +63,8 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { } log.Debug().Msg("processing transactions from proposal block") - mempool := uow.GetBus().GetUtilityModule().GetMempool() - if err := uow.processTransactionsFromProposalBlock(mempool, uow.proposalBlockTxs); err != nil { + txMempool := uow.GetBus().GetUtilityModule().GetMempool() + if err := uow.processTransactionsFromProposalBlock(txMempool, uow.proposalBlockTxs); err != nil { return "", nil, err } @@ -76,7 +76,7 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() (string, [][]byte, error) { // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // return the app hash (consensus module will get the validator set directly) log.Debug().Msg("computing state hash") - stateHash, err := uow.persistenceRWContext.ComputeStateHash() + stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") return "", nil, utilTypes.ErrAppHash(err) @@ -120,7 +120,7 @@ func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { // processTransactionsFromProposalBlock processes the transactions from the proposal block. // It also removes the transactions from the mempool if they are already present. -func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(mempool mempool.TXMempool, txsBytes [][]byte) (err error) { +func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool mempool.TXMempool, txsBytes [][]byte) (err error) { for index, txProtoBytes := range txsBytes { tx, err := coreTypes.TxFromBytes(txProtoBytes) if err != nil { @@ -140,8 +140,8 @@ func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(mempool m return err } - if mempool.Contains(txHash) { - if err := mempool.RemoveTx(txProtoBytes); err != nil { + if txMempool.Contains(txHash) { + if err := txMempool.RemoveTx(txProtoBytes); err != nil { return err } uow.logger.Info().Str("tx_hash", txHash).Msg("Applying tx that WAS in the local mempool") diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 99975df45..af278dbb7 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -66,14 +66,14 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy } // reapMempool reaps transactions from the mempool up to the maximum transaction bytes allowed in a block. -func (uow *leaderUtilityUnitOfWork) reapMempool(mempool mempool.TXMempool, maxTxBytes uint64) (txs [][]byte, err error) { +func (uow *leaderUtilityUnitOfWork) reapMempool(txMempool mempool.TXMempool, maxTxBytes uint64) (txs [][]byte, err error) { txs = make([][]byte, 0) txsTotalBz := uint64(0) txIdx := 0 - for !mempool.IsEmpty() { + for !txMempool.IsEmpty() { // NB: In order for transactions to have entered the mempool, `HandleTransaction` must have // been called which handles basic checks & validation. - txBz, err := mempool.PopTx() + txBz, err := txMempool.PopTx() if err != nil { return nil, err } @@ -89,7 +89,7 @@ func (uow *leaderUtilityUnitOfWork) reapMempool(mempool mempool.TXMempool, maxTx // Exceeding maximum transaction bytes to be added in this block if txsTotalBz >= maxTxBytes { // Add back popped tx to be applied in a future block - if err := mempool.AddTx(txBz); err != nil { + if err := txMempool.AddTx(txBz); err != nil { return nil, err } break // we've reached our max From f092dcd17501dfcb1376f815c09a480a63b1f6d2 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 5 Apr 2023 00:09:38 +0100 Subject: [PATCH 13/30] chore(utility): lint --- utility/unit_of_work/uow_leader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index af278dbb7..2df59c2e4 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -42,8 +42,8 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy } log.Debug().Msg("reaping the mempool") - mempool := uow.GetBus().GetUtilityModule().GetMempool() - if txs, err = uow.reapMempool(mempool, maxTxBytes); err != nil { + txMempool := uow.GetBus().GetUtilityModule().GetMempool() + if txs, err = uow.reapMempool(txMempool, maxTxBytes); err != nil { return "", nil, err } From 657841c5cf19be479c85382d503c7aef42236410 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 22:02:15 +0100 Subject: [PATCH 14/30] chore(shared): nits --- shared/modules/utility_module.go | 3 --- utility/unit_of_work/module.go | 6 +++--- utility/unit_of_work/uow_leader.go | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/shared/modules/utility_module.go b/shared/modules/utility_module.go index 3d2923155..aef9d82a5 100644 --- a/shared/modules/utility_module.go +++ b/shared/modules/utility_module.go @@ -42,9 +42,6 @@ type UnstakingActor interface { // CONSIDERATION: Consider removing `Utility` from `UtilityUnitOfWork` altogether -// TECHDEBT(@deblasis): `CreateProposalBlock` and `ApplyBlock` should be be refactored into a -// `GetProposalBlock` and `ApplyProposalBlock` functions - // UtilityUnitOfWork is a unit of work (https://martinfowler.com/eaaCatalog/unitOfWork.html) that allows for atomicity and commit/rollback functionality type UtilityUnitOfWork interface { IntegratableModule diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 975024613..1c5610642 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -45,7 +45,6 @@ func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAdd return nil } -// CLEANUP: code re-use ApplyBlock() for CreateAndApplyBlock() func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, err error) { log := uow.logger.With().Fields(map[string]interface{}{ "source": "ApplyBlock", @@ -73,7 +72,6 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, if err := uow.endBlock(uow.proposalProposerAddr); err != nil { return "", nil, err } - // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // return the app hash (consensus module will get the validator set directly) log.Debug().Msg("computing state hash") stateHash, err = uow.persistenceRWContext.ComputeStateHash() @@ -114,12 +112,14 @@ func (uow *baseUtilityUnitOfWork) Release() error { return nil } +// isProposalBlockSet returns true if the proposal block has been set. +// TODO: it should also check that uow.proposalBlockTxs is not empty but if we do, tests fail. func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { return uow.proposalStateHash != "" && uow.proposalProposerAddr != nil } // processTransactionsFromProposalBlock processes the transactions from the proposal block. -// It also removes the transactions from the mempool if they are already present. +// It also removes the transactions from the mempool if they are also present. func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool mempool.TXMempool, txsBytes [][]byte) (err error) { for index, txProtoBytes := range txsBytes { tx, err := coreTypes.TxFromBytes(txProtoBytes) diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 2df59c2e4..980e6ffd1 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -58,7 +58,7 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy // Compute & return the new state hash stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { - log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + log.Fatal().Bool("TODO", true).Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") } log.Info().Str("state_hash", stateHash).Msgf("Finished successfully") @@ -109,7 +109,7 @@ func (uow *leaderUtilityUnitOfWork) reapMempool(txMempool mempool.TXMempool, max // TODO(#564): make sure that indexing is reversible in case of a rollback // Index the transaction if err := uow.persistenceRWContext.IndexTransaction(txResult); err != nil { - uow.logger.Fatal().Err(err).Msgf("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now: %v\n", err) + uow.logger.Fatal().Bool("TODO", true).Err(err).Msgf("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now: %v\n", err) } txs = append(txs, txBz) From 9a54d7654c86be83aea12bb34cd38e6801c6ae10 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 22:03:25 +0100 Subject: [PATCH 15/30] Update shared/modules/utility_module.go Co-authored-by: Daniel Olshansky --- shared/modules/utility_module.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shared/modules/utility_module.go b/shared/modules/utility_module.go index 3d2923155..6741e1318 100644 --- a/shared/modules/utility_module.go +++ b/shared/modules/utility_module.go @@ -70,8 +70,7 @@ type UtilityUnitOfWork interface { type LeaderUtilityUnitOfWork interface { UtilityUnitOfWork - // CreateProposalBlock reaps the mempool for txs to be proposed in a new block, and - // applies them to this context after validation. + // CreateProposalBlock reaps the mempool for txs to be proposed in a new block. CreateProposalBlock(proposer []byte, maxTxBytes uint64) (stateHash string, txs [][]byte, err error) } From 239bc729b59b16b1d700d115389e6ca9991e4b3d Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 22:03:50 +0100 Subject: [PATCH 16/30] Update utility/unit_of_work/module.go Co-authored-by: Daniel Olshansky --- utility/unit_of_work/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 975024613..8ee7ed4e6 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -119,7 +119,7 @@ func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { } // processTransactionsFromProposalBlock processes the transactions from the proposal block. -// It also removes the transactions from the mempool if they are already present. +// It also removes the transactions from the mempool if they are also present. func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool mempool.TXMempool, txsBytes [][]byte) (err error) { for index, txProtoBytes := range txsBytes { tx, err := coreTypes.TxFromBytes(txProtoBytes) From c80c0fde66482edbf34345e00040d27d6b7df959 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 22:04:22 +0100 Subject: [PATCH 17/30] Update utility/unit_of_work/uow_leader.go Co-authored-by: Daniel Olshansky --- utility/unit_of_work/uow_leader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 2df59c2e4..9c6638a93 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -60,7 +60,7 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy if err != nil { log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") } - log.Info().Str("state_hash", stateHash).Msgf("Finished successfully") + log.Info().Str("state_hash", stateHash).Msg("Finished successfully") return stateHash, txs, err } From c3828cf5083a914a041387f5e2368112d353f7fb Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 22:06:19 +0100 Subject: [PATCH 18/30] chore(utility): nits --- utility/unit_of_work/uow_leader.go | 1 - 1 file changed, 1 deletion(-) diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 980e6ffd1..3216f523f 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -54,7 +54,6 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy } log.Debug().Msg("computing state hash") - // TODO(@deblasis): this should be from a ReadContext (the ephemeral/staging one) // Compute & return the new state hash stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { From 9b8c85fca9317adb316fd53ac0fb5e5fa8a163d0 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Thu, 6 Apr 2023 23:43:46 +0100 Subject: [PATCH 19/30] feat(utility): state hash check with feature flag for tests --- utility/unit_of_work/block.go | 4 ++++ utility/unit_of_work/block_test.go | 6 +++--- utility/unit_of_work/module.go | 19 +++++++++++++++++-- utility/unit_of_work/uow_leader.go | 4 ++-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/utility/unit_of_work/block.go b/utility/unit_of_work/block.go index 2fc85c839..95bad4890 100644 --- a/utility/unit_of_work/block.go +++ b/utility/unit_of_work/block.go @@ -12,6 +12,10 @@ import ( typesUtil "github.com/pokt-network/pocket/utility/types" ) +const ( + IgnoreProposalBlockCheckHash = "0100000000000000000000000000000000000000000000000000000000000010" +) + func (uow *baseUtilityUnitOfWork) beginBlock() typesUtil.Error { log := uow.logger.With().Fields(map[string]interface{}{ "source": "beginBlock", diff --git a/utility/unit_of_work/block_test.go b/utility/unit_of_work/block_test.go index b416feaa1..9b5094472 100644 --- a/utility/unit_of_work/block_test.go +++ b/utility/unit_of_work/block_test.go @@ -40,7 +40,7 @@ func TestUtilityUnitOfWork_ApplyBlock(t *testing.T) { _, _, err = uow.ApplyBlock() require.Equal(t, err.Error(), utilTypes.ErrProposalBlockNotSet().Error()) - err = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) + err = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, err) appHash, _, err := uow.ApplyBlock() @@ -89,7 +89,7 @@ func TestUtilityUnitOfWork_BeginBlock(t *testing.T) { addrBz, er := hex.DecodeString(proposer.GetAddress()) require.NoError(t, er) - er = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) + er = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, er) _, _, er = uow.ApplyBlock() @@ -117,7 +117,7 @@ func TestUtilityUnitOfWork_EndBlock(t *testing.T) { proposerBeforeBalance, err := uow.getAccountAmount(addrBz) require.NoError(t, err) - er = uow.SetProposalBlock("computed_state_hash_placeholder", addrBz, [][]byte{txBz}) + er = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, er) _, _, er = uow.ApplyBlock() diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 1c5610642..641acfd81 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -1,6 +1,8 @@ package unit_of_work import ( + "fmt" + coreTypes "github.com/pokt-network/pocket/shared/core/types" "github.com/pokt-network/pocket/shared/mempool" "github.com/pokt-network/pocket/shared/modules" @@ -76,9 +78,22 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, log.Debug().Msg("computing state hash") stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { - log.Fatal().Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + log.Fatal().Err(err).Bool("TODO", true).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") return "", nil, utilTypes.ErrAppHash(err) } + + // IMPROVE: this acts as a feature flag to allow tests to ignore the check if needed, ideally the tests should have a way to determine + // the hash and set it into the proposal block it's currently hard to do because the state is different at every test run (non-determinism) + if uow.proposalStateHash != IgnoreProposalBlockCheckHash { + if uow.proposalStateHash != stateHash { + log.Fatal().Bool("TODO", true). + Str("proposalStateHash", uow.proposalStateHash). + Str("stateHash", stateHash). + Msg("State hash mismatch. TODO: Look into roll-backing the entire commit...") + return "", nil, utilTypes.ErrAppHash(fmt.Errorf("state hash mismatch: expected %s from the proposal, got %s", uow.proposalStateHash, stateHash)) + } + } + log.Info().Str("state_hash", stateHash).Msgf("ApplyBlock succeeded!") // return the app hash; consensus module will get the validator set directly @@ -151,7 +166,7 @@ func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool // TODO(#564): make sure that indexing is reversible in case of a rollback if err := uow.persistenceRWContext.IndexTransaction(txResult); err != nil { - uow.logger.Fatal().Err(err).Msgf("TODO(#327): We can apply the transaction but not index it. Crash the process for now: %v\n", err) + uow.logger.Fatal().Err(err).Msg("TODO(#327): We can apply the transaction but not index it. Crash the process for now") } } return nil diff --git a/utility/unit_of_work/uow_leader.go b/utility/unit_of_work/uow_leader.go index 63215b6a2..f4a487601 100644 --- a/utility/unit_of_work/uow_leader.go +++ b/utility/unit_of_work/uow_leader.go @@ -57,7 +57,7 @@ func (uow *leaderUtilityUnitOfWork) CreateProposalBlock(proposer []byte, maxTxBy // Compute & return the new state hash stateHash, err = uow.persistenceRWContext.ComputeStateHash() if err != nil { - log.Fatal().Bool("TODO", true).Err(err).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") + log.Fatal().Err(err).Bool("TODO", true).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") } log.Info().Str("state_hash", stateHash).Msg("Finished successfully") @@ -108,7 +108,7 @@ func (uow *leaderUtilityUnitOfWork) reapMempool(txMempool mempool.TXMempool, max // TODO(#564): make sure that indexing is reversible in case of a rollback // Index the transaction if err := uow.persistenceRWContext.IndexTransaction(txResult); err != nil { - uow.logger.Fatal().Bool("TODO", true).Err(err).Msgf("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now: %v\n", err) + uow.logger.Fatal().Bool("TODO", true).Err(err).Msg("TODO(#327): The transaction can by hydrated but not indexed. Crash the process for now") } txs = append(txs, txBz) From 5c625eef55b60d5642fc8e422c572dd8fc9cf2bd Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:03:37 +0100 Subject: [PATCH 20/30] chore(consensus): tidy duplicate import --- consensus/hotstuff_replica.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/consensus/hotstuff_replica.go b/consensus/hotstuff_replica.go index b243ae6a3..c1aa7d1cb 100644 --- a/consensus/hotstuff_replica.go +++ b/consensus/hotstuff_replica.go @@ -4,7 +4,6 @@ import ( "fmt" consensusTelemetry "github.com/pokt-network/pocket/consensus/telemetry" - "github.com/pokt-network/pocket/consensus/types" typesCons "github.com/pokt-network/pocket/consensus/types" "github.com/pokt-network/pocket/shared/codec" coreTypes "github.com/pokt-network/pocket/shared/core/types" @@ -310,20 +309,20 @@ func (m *consensusModule) validateQuorumCertificate(qc *typesCons.QuorumCertific return nil } -func isNodeLockedOnPastQC(justifyQC, lockedQC *types.QuorumCertificate) (bool, error) { +func isNodeLockedOnPastQC(justifyQC, lockedQC *typesCons.QuorumCertificate) (bool, error) { if isLockedOnPastHeight(justifyQC, lockedQC) { - return true, types.ErrNodeLockedPastHeight + return true, typesCons.ErrNodeLockedPastHeight } else if isLockedOnCurrHeightAndPastRound(justifyQC, lockedQC) { - return true, types.ErrNodeLockedPastHeight + return true, typesCons.ErrNodeLockedPastHeight } return false, nil } -func isLockedOnPastHeight(justifyQC, lockedQC *types.QuorumCertificate) bool { +func isLockedOnPastHeight(justifyQC, lockedQC *typesCons.QuorumCertificate) bool { return justifyQC.Height > lockedQC.Height } -func isLockedOnCurrHeightAndPastRound(justifyQC, lockedQC *types.QuorumCertificate) bool { +func isLockedOnCurrHeightAndPastRound(justifyQC, lockedQC *typesCons.QuorumCertificate) bool { return justifyQC.Height == lockedQC.Height && justifyQC.Round > lockedQC.Round } From 4be8bf754d45fa4f860dc30c1f819db315bc613b Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:04:15 +0100 Subject: [PATCH 21/30] refactor(consensus): updated to reflect changes in utility --- consensus/e2e_tests/utils_test.go | 12 ++++++++++-- consensus/hotstuff_replica.go | 6 ++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 25034b028..8f22ef128 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -464,7 +464,11 @@ func baseLeaderUtilityUnitOfWorkMock(t *testing.T, genesisState *genesis.Genesis AnyTimes() utilityLeaderUnitOfWorkMock.EXPECT(). ApplyBlock(). - Return(stateHash, make([][]byte, 0), nil). + Return(nil). + AnyTimes() + utilityLeaderUnitOfWorkMock.EXPECT(). + GetStateHash(). + Return(stateHash). AnyTimes() utilityLeaderUnitOfWorkMock.EXPECT().SetProposalBlock(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() utilityLeaderUnitOfWorkMock.EXPECT().Commit(gomock.Any()).Return(nil).AnyTimes() @@ -484,7 +488,11 @@ func baseReplicaUtilityUnitOfWorkMock(t *testing.T, genesisState *genesis.Genesi utilityReplicaUnitOfWorkMock.EXPECT(). ApplyBlock(). - Return(stateHash, make([][]byte, 0), nil). + Return(nil). + AnyTimes() + utilityReplicaUnitOfWorkMock.EXPECT(). + GetStateHash(). + Return(stateHash). AnyTimes() utilityReplicaUnitOfWorkMock.EXPECT().SetProposalBlock(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() utilityReplicaUnitOfWorkMock.EXPECT().Commit(gomock.Any()).Return(nil).AnyTimes() diff --git a/consensus/hotstuff_replica.go b/consensus/hotstuff_replica.go index c1aa7d1cb..80b25cee7 100644 --- a/consensus/hotstuff_replica.go +++ b/consensus/hotstuff_replica.go @@ -246,12 +246,14 @@ func (m *consensusModule) applyBlock(block *coreTypes.Block) error { return err } - // Apply all the transactions in the block and get the stateHash - stateHash, _, err := utilityUnitOfWork.ApplyBlock() + // Apply all the transactions in the block + err := utilityUnitOfWork.ApplyBlock() if err != nil { return err } + stateHash := utilityUnitOfWork.GetStateHash() + if blockHeader.StateHash != stateHash { return typesCons.ErrInvalidStateHash(blockHeader.StateHash, stateHash) } From 391db1730aa5e05e5dd4e465ef8ac3788a5b9bbc Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:04:50 +0100 Subject: [PATCH 22/30] feat(shared): utilityUOW.GetStateHash() --- shared/modules/utility_module.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shared/modules/utility_module.go b/shared/modules/utility_module.go index b4dfbe4f1..068b81cc8 100644 --- a/shared/modules/utility_module.go +++ b/shared/modules/utility_module.go @@ -55,13 +55,16 @@ type UtilityUnitOfWork interface { // ApplyBlock applies the context's in-memory proposed state (i.e. the txs in this context). // Only intended to be used by the block verifiers (i.e. replicas). // NOTE: this is called by the replica OR by the leader when `prepareQc` is not `nil` - ApplyBlock() (stateHash string, txs [][]byte, err error) + ApplyBlock() error // Release releases this utility unit of work and any underlying contexts it references Release() error // Commit commits this utility unit of work along with any underlying contexts (e.g. persistenceContext) it references Commit(quorumCert []byte) error + + // GetStateHash returns the state hash of the current utility unit of work + GetStateHash() string } type LeaderUtilityUnitOfWork interface { From 9208678e33f3dba104e581bd78dbe30a66ce05bf Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:05:14 +0100 Subject: [PATCH 23/30] feat(utility): GetStateHash() implementation --- utility/unit_of_work/block_test.go | 11 ++++++----- utility/unit_of_work/module.go | 28 ++++++++++++++++++---------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/utility/unit_of_work/block_test.go b/utility/unit_of_work/block_test.go index 9b5094472..e427ac9b9 100644 --- a/utility/unit_of_work/block_test.go +++ b/utility/unit_of_work/block_test.go @@ -37,15 +37,16 @@ func TestUtilityUnitOfWork_ApplyBlock(t *testing.T) { require.NoError(t, err) // calling ApplyBlock without having called SetProposalBlock first should fail with ErrProposalBlockNotSet - _, _, err = uow.ApplyBlock() + err = uow.ApplyBlock() require.Equal(t, err.Error(), utilTypes.ErrProposalBlockNotSet().Error()) err = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, err) - appHash, _, err := uow.ApplyBlock() + err = uow.ApplyBlock() + stateHash := uow.GetStateHash() require.NoError(t, err) - require.NotNil(t, appHash) + require.NotNil(t, stateHash) // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. // beginBlock logic verify @@ -92,7 +93,7 @@ func TestUtilityUnitOfWork_BeginBlock(t *testing.T) { er = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, er) - _, _, er = uow.ApplyBlock() + er = uow.ApplyBlock() require.NoError(t, er) // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. @@ -120,7 +121,7 @@ func TestUtilityUnitOfWork_EndBlock(t *testing.T) { er = uow.SetProposalBlock(IgnoreProposalBlockCheckHash, addrBz, [][]byte{txBz}) require.NoError(t, er) - _, _, er = uow.ApplyBlock() + er = uow.ApplyBlock() require.NoError(t, er) feeBig, err := uow.getMessageSendFee() diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 641acfd81..0a5095f3f 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -38,6 +38,8 @@ type baseUtilityUnitOfWork struct { proposalStateHash string proposalProposerAddr []byte proposalBlockTxs [][]byte + + stateHash string } func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAddr []byte, txs [][]byte) error { @@ -47,39 +49,39 @@ func (uow *baseUtilityUnitOfWork) SetProposalBlock(blockHash string, proposerAdd return nil } -func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, err error) { +func (uow *baseUtilityUnitOfWork) ApplyBlock() error { log := uow.logger.With().Fields(map[string]interface{}{ "source": "ApplyBlock", }).Logger() log.Debug().Msg("checking if proposal block has been set") if !uow.isProposalBlockSet() { - return "", nil, utilTypes.ErrProposalBlockNotSet() + return utilTypes.ErrProposalBlockNotSet() } // begin block lifecycle phase log.Debug().Msg("calling beginBlock") if err := uow.beginBlock(); err != nil { - return "", nil, err + return err } log.Debug().Msg("processing transactions from proposal block") txMempool := uow.GetBus().GetUtilityModule().GetMempool() if err := uow.processTransactionsFromProposalBlock(txMempool, uow.proposalBlockTxs); err != nil { - return "", nil, err + return err } // end block lifecycle phase log.Debug().Msg("calling endBlock") if err := uow.endBlock(uow.proposalProposerAddr); err != nil { - return "", nil, err + return err } // return the app hash (consensus module will get the validator set directly) log.Debug().Msg("computing state hash") - stateHash, err = uow.persistenceRWContext.ComputeStateHash() + stateHash, err := uow.persistenceRWContext.ComputeStateHash() if err != nil { log.Fatal().Err(err).Bool("TODO", true).Msg("Updating the app hash failed. TODO: Look into roll-backing the entire commit...") - return "", nil, utilTypes.ErrAppHash(err) + return utilTypes.ErrAppHash(err) } // IMPROVE: this acts as a feature flag to allow tests to ignore the check if needed, ideally the tests should have a way to determine @@ -90,14 +92,15 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() (stateHash string, txs [][]byte, Str("proposalStateHash", uow.proposalStateHash). Str("stateHash", stateHash). Msg("State hash mismatch. TODO: Look into roll-backing the entire commit...") - return "", nil, utilTypes.ErrAppHash(fmt.Errorf("state hash mismatch: expected %s from the proposal, got %s", uow.proposalStateHash, stateHash)) + return utilTypes.ErrAppHash(fmt.Errorf("state hash mismatch: expected %s from the proposal, got %s", uow.proposalStateHash, stateHash)) } } log.Info().Str("state_hash", stateHash).Msgf("ApplyBlock succeeded!") - // return the app hash; consensus module will get the validator set directly - return stateHash, nil, nil + uow.stateHash = stateHash + + return nil } // TODO(@deblasis): change tracking here @@ -171,3 +174,8 @@ func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool } return nil } + +// GetStateHash returns the state hash of the unit of work. It is only available after the block has been applied. +func (uow *baseUtilityUnitOfWork) GetStateHash() string { + return uow.stateHash +} From ed49d939b6f6a7e1b91c6c63a0a7eec59937ed4f Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:10:56 +0100 Subject: [PATCH 24/30] refactor(utility): processTransactionsFromProposalBlock signature --- utility/unit_of_work/module.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index 0a5095f3f..e414af963 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -67,7 +67,7 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() error { log.Debug().Msg("processing transactions from proposal block") txMempool := uow.GetBus().GetUtilityModule().GetMempool() - if err := uow.processTransactionsFromProposalBlock(txMempool, uow.proposalBlockTxs); err != nil { + if err := uow.processTransactionsFromProposalBlock(txMempool); err != nil { return err } @@ -138,8 +138,8 @@ func (uow *baseUtilityUnitOfWork) isProposalBlockSet() bool { // processTransactionsFromProposalBlock processes the transactions from the proposal block. // It also removes the transactions from the mempool if they are also present. -func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool mempool.TXMempool, txsBytes [][]byte) (err error) { - for index, txProtoBytes := range txsBytes { +func (uow *baseUtilityUnitOfWork) processTransactionsFromProposalBlock(txMempool mempool.TXMempool) (err error) { + for index, txProtoBytes := range uow.proposalBlockTxs { tx, err := coreTypes.TxFromBytes(txProtoBytes) if err != nil { return err From d96bb0e2faadb17f218c5cb639200ba651ac296d Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:13:43 +0100 Subject: [PATCH 25/30] docs(consensus): CHANGELOG --- consensus/doc/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index bc1d76bb1..e95eaa237 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.44] - 2023-04-07 + +- Updated to reflect the new `ApplyBlock` signature from `utilityUnitOfWork` +- Updated to use `utilityUnitOfWork.GetStateHash()` +- Removed duplicate import + ## [0.0.0.43] - 2023-04-04 - Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` From 7f5df10219184d45d66f471cb7d2562bdeac3fba Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:16:45 +0100 Subject: [PATCH 26/30] h5law Signed-off-by: Alessandro De Blasis --- shared/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shared/CHANGELOG.md b/shared/CHANGELOG.md index 780ea447f..861228a4a 100644 --- a/shared/CHANGELOG.md +++ b/shared/CHANGELOG.md @@ -7,9 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.48] - 2023-04-06 +## [0.0.0.48] - 2023-04-07 - Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` +- Added `GetStateHash` to `UtilityUnitOfWork` ## [0.0.0.47] - 2023-04-06 From 1911fb1ad6f4c2e0fa0214ff39318e26c3a0d201 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:17:33 +0100 Subject: [PATCH 27/30] docs(utility): CHANGELOG --- utility/doc/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/utility/doc/CHANGELOG.md b/utility/doc/CHANGELOG.md index d360a58cb..c8cd669b3 100644 --- a/utility/doc/CHANGELOG.md +++ b/utility/doc/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.34] - 2023-04-06 +## [0.0.0.35] - 2023-04-06 - Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` - Added `GetPrevBlockByzantineValidators` and `ProposalBlockNotSet` errors @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Refactored functions for block creation and application to be more readable/modular - Added TODOs for future refactoring - Renamed `u` to `uow` for consistency +- Added `stateHash` validation against `proposalBlock` +- Added feature flag for `stateHash` validation (for testing purposes) +- Updated tests accordingly ## [0.0.0.34] - 2023-04-06 From abbe4e667045a4f498cf7fbfb8ab30548c8f7f8f Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:20:10 +0100 Subject: [PATCH 28/30] chore(utility): CHANGELOG date --- utility/doc/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/doc/CHANGELOG.md b/utility/doc/CHANGELOG.md index c8cd669b3..6d00caf16 100644 --- a/utility/doc/CHANGELOG.md +++ b/utility/doc/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.35] - 2023-04-06 +## [0.0.0.35] - 2023-04-07 - Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` - Added `GetPrevBlockByzantineValidators` and `ProposalBlockNotSet` errors From 3ecdc2388670ee7a1d9acfc10f8e84d0fe2bb057 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 00:53:39 +0100 Subject: [PATCH 29/30] Update utility/unit_of_work/module.go Co-authored-by: Daniel Olshansky --- utility/unit_of_work/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility/unit_of_work/module.go b/utility/unit_of_work/module.go index e414af963..3bf503ea4 100644 --- a/utility/unit_of_work/module.go +++ b/utility/unit_of_work/module.go @@ -84,7 +84,7 @@ func (uow *baseUtilityUnitOfWork) ApplyBlock() error { return utilTypes.ErrAppHash(err) } - // IMPROVE: this acts as a feature flag to allow tests to ignore the check if needed, ideally the tests should have a way to determine + // IMPROVE(#655): this acts as a feature flag to allow tests to ignore the check if needed, ideally the tests should have a way to determine // the hash and set it into the proposal block it's currently hard to do because the state is different at every test run (non-determinism) if uow.proposalStateHash != IgnoreProposalBlockCheckHash { if uow.proposalStateHash != stateHash { From d17a865ebf9f07c92e868d1dbe71619b49326411 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Fri, 7 Apr 2023 01:02:15 +0100 Subject: [PATCH 30/30] docs(consensus): CHANGELOG --- consensus/doc/CHANGELOG.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index e95eaa237..7994f97f5 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,16 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.44] - 2023-04-07 +## [0.0.0.43] - 2023-04-07 +- Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` - Updated to reflect the new `ApplyBlock` signature from `utilityUnitOfWork` - Updated to use `utilityUnitOfWork.GetStateHash()` - Removed duplicate import -## [0.0.0.43] - 2023-04-04 - -- Renamed `CreateAndApplyProposalBlock` to `CreateProposalBlock` - ## [0.0.0.42] - 2023-04-03 - Add `fsm_handler.go` to handle FSM transition events in consensus module