-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core/types,node/types: add cached tx hash to ktypes.Transaction, create node/types.Tx #1422
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,7 @@ func (ce *ConsensusEngine) validateBlock(blk *ktypes.Block) error { | |
} | ||
|
||
// Verify the merkle root of the block transactions | ||
merkleRoot := blk.MerkleRoot() | ||
merkleRoot := blk.CalcMerkleRoot() // NOTE: this expects CalcMerkleRoot to use tx.HashCache() to prepare the Transaction's internal hash cache | ||
if merkleRoot != blk.Header.MerkleRoot { | ||
return fmt.Errorf("merkleroot mismatch, expected %v, got %v", merkleRoot, blk.Header.MerkleRoot) | ||
} | ||
|
@@ -117,14 +117,13 @@ func (ce *ConsensusEngine) lastBlock() (int64, types.Hash, time.Time) { | |
// It is an error if the transaction is already in the mempool. | ||
// It is an error if the transaction fails CheckTx. | ||
// This method holds the mempool lock for the duration of the call. | ||
func (ce *ConsensusEngine) QueueTx(ctx context.Context, tx *ktypes.Transaction) error { | ||
func (ce *ConsensusEngine) QueueTx(ctx context.Context, tx *types.Tx) error { | ||
height, _, timestamp := ce.lastBlock() | ||
|
||
ce.mempoolMtx.Lock() | ||
defer ce.mempoolMtx.Unlock() | ||
|
||
txHash := tx.Hash() | ||
have, rejected := ce.mempool.Store(txHash, tx) | ||
Comment on lines
-126
to
-127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This silly optimization of having |
||
have, rejected := ce.mempool.Store(tx) | ||
if have { | ||
return ktypes.ErrTxAlreadyExists | ||
} | ||
|
@@ -135,7 +134,7 @@ func (ce *ConsensusEngine) QueueTx(ctx context.Context, tx *ktypes.Transaction) | |
const recheck = false | ||
err := ce.blockProcessor.CheckTx(ctx, tx, height, timestamp, recheck) | ||
if err != nil { | ||
ce.mempool.Remove(txHash) | ||
ce.mempool.Remove(tx.Hash()) | ||
return err | ||
} | ||
|
||
|
@@ -181,9 +180,9 @@ func (ce *ConsensusEngine) lastBlockInternal() (int64, time.Time) { | |
|
||
// recheckTxFn creates a tx recheck function for the mempool that assumes the | ||
// provided height and timestamp for each call. | ||
func (ce *ConsensusEngine) recheckTxFn(height int64, timestamp time.Time) func(ctx context.Context, tx *ktypes.Transaction) error { | ||
func (ce *ConsensusEngine) recheckTxFn(height int64, timestamp time.Time) func(ctx context.Context, tx *types.Tx) error { | ||
// height, _, timestamp := ce.lastBlock() | ||
return func(ctx context.Context, tx *ktypes.Transaction) error { | ||
return func(ctx context.Context, tx *types.Tx) error { | ||
const recheck = true | ||
return ce.blockProcessor.CheckTx(ctx, tx, height, timestamp, recheck) | ||
} | ||
|
@@ -192,8 +191,10 @@ func (ce *ConsensusEngine) recheckTxFn(height int64, timestamp time.Time) func(c | |
// BroadcastTx checks the transaction with the mempool and if the verification | ||
// is successful, broadcasts it to the network. The TxResult will be nil unless | ||
// sync is set to 1, in which case the BroadcastTx returns only after it is | ||
// successfully executed in a committed block. | ||
func (ce *ConsensusEngine) BroadcastTx(ctx context.Context, tx *ktypes.Transaction, sync uint8) (types.Hash, *ktypes.TxResult, error) { | ||
// successfully executed in a committed block. This method is effectively | ||
// [QueueTx] followed, by P2P broadcast of the transaction, followed by | ||
// optionally waiting for the transaction to be mined. | ||
func (ce *ConsensusEngine) BroadcastTx(ctx context.Context, tx *types.Tx, sync uint8) (types.Hash, *ktypes.TxResult, error) { | ||
// check and store the transaction in the mempool | ||
if err := ce.QueueTx(ctx, tx); err != nil { | ||
return types.Hash{}, nil, err | ||
|
@@ -205,7 +206,7 @@ func (ce *ConsensusEngine) BroadcastTx(ctx context.Context, tx *ktypes.Transacti | |
if ce.txAnnouncer != nil { | ||
// We can't use parent context 'cause it's canceled in the caller, which | ||
// could be the RPC request. handler. This shouldn't be CE's problem... | ||
go ce.txAnnouncer(context.Background(), tx) | ||
go ce.txAnnouncer(context.Background(), tx.Transaction) | ||
} | ||
|
||
// If sync is set to 1, wait for the transaction to be committed in a block. | ||
|
@@ -301,7 +302,7 @@ func (ce *ConsensusEngine) commit(ctx context.Context) error { | |
|
||
// remove transactions from the mempool | ||
for idx, txn := range blkProp.blk.Txns { | ||
txHash := txn.Hash() | ||
txHash := txn.HashCache() | ||
ce.mempool.Remove(txHash) | ||
|
||
txRes := ce.state.blockRes.txResults[idx] | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateBlock
is a key method that is one of the first points at which a node will begin processing a block it has received, e.g. from p2p, block store, etc. Ensuring that this method warms the internal tx hash cached incore/types.Transaction
benefits many other spots downstream.