address reveiw

This commit is contained in:
D-Stacks 2022-07-22 21:01:26 +02:00
parent 79c934e39c
commit 762ea907c4
14 changed files with 58 additions and 122 deletions

View File

@ -30,10 +30,6 @@ func (flow *handleRequestedTransactionsFlow) start() error {
}
for _, transactionID := range msgRequestTransactions.IDs {
//note: below ignores orphan txs that are requested
//find out if this is good or bad practice
//only reference i found to this, is that nodes don't do this in btc
//source: https://arxiv.org/abs/1912.11541 (2nd sentence in abstract)
tx, _, ok := flow.Domain().MiningManager().GetTransaction(transactionID, true, false)
if !ok {

View File

@ -6,7 +6,6 @@ import (
"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
"github.com/kaspanet/kaspad/domain/consensus/utils/transactionid"
"github.com/kaspanet/kaspad/infrastructure/network/netadapter/router"
"github.com/pkg/errors"
)
// HandleGetMempoolEntry handles the respectively named RPC command
@ -25,25 +24,15 @@ func HandleGetMempoolEntry(context *rpccontext.Context, _ *router.Router, reques
return errorMessage, nil
}
transactionPoolTransaction, orphanPoolTransaction, found := context.Domain.MiningManager().GetTransaction(transactionID, !getMempoolEntryRequest.FilterTransactionPool, getMempoolEntryRequest.IncludeOrphanPool)
mempoolTransaction, isOrphan, found := context.Domain.MiningManager().GetTransaction(transactionID, !getMempoolEntryRequest.FilterTransactionPool, getMempoolEntryRequest.IncludeOrphanPool)
if !found {
if !found || isOrphan && !getMempoolEntryRequest.IncludeOrphanPool || !isOrphan && !getMempoolEntryRequest.FilterTransactionPool {
errorMessage := &appmessage.GetMempoolEntryResponseMessage{}
errorMessage.Error = appmessage.RPCErrorf("Transaction %s was not found", transactionID)
return errorMessage, nil
}
if transactionPoolTransaction != nil && orphanPoolTransaction != nil {
return nil, errors.Errorf("Transaction %s is both an orphan, and not. This should never not happen", transactionID)
} else if transactionPoolTransaction != nil {
transaction = transactionPoolTransaction
isOrphan = false
} else if orphanPoolTransaction != nil {
transaction = orphanPoolTransaction
isOrphan = true
}
rpcTransaction := appmessage.DomainTransactionToRPCTransaction(transaction)
rpcTransaction := appmessage.DomainTransactionToRPCTransaction(mempoolTransaction)
err = context.PopulateTransactionWithVerboseData(rpcTransaction, nil)
if err != nil {
return nil, err

View File

@ -20,7 +20,7 @@ type factory struct{}
func (f *factory) NewMiningManager(consensusReference consensusreference.ConsensusReference, params *dagconfig.Params,
mempoolConfig *mempoolpkg.Config) MiningManager {
mempool := mempoolpkg.New(mempoolConfig, params, consensusReference)
mempool := mempoolpkg.New(mempoolConfig, consensusReference)
blockTemplateBuilder := blocktemplatebuilder.New(consensusReference, mempool, params.MaxBlockMass, params.CoinbasePayloadScriptPublicKeyMaxLength)
return &miningManager{

View File

@ -95,7 +95,7 @@ func TestCalcMinRequiredTxRelayFee(t *testing.T) {
mempoolConfig.MinimumRelayTransactionFee = test.minimumRelayTransactionFee
tcAsConsensus := tc.(externalapi.Consensus)
tcAsConsensusPointer := &tcAsConsensus
mempool := New(mempoolConfig, tc.DAGParams(), consensusreference.NewConsensusReference(&tcAsConsensusPointer)).(*mempool)
mempool := New(mempoolConfig, consensusreference.NewConsensusReference(&tcAsConsensusPointer)).(*mempool)
got := mempool.minimumRequiredTransactionRelayFee(test.size)
if got != test.want {
@ -184,7 +184,7 @@ func TestIsTransactionOutputDust(t *testing.T) {
mempoolConfig.MinimumRelayTransactionFee = test.minimumRelayTransactionFee
tcAsConsensus := tc.(externalapi.Consensus)
tcAsConsensusPointer := &tcAsConsensus
mempool := New(mempoolConfig, tc.DAGParams(), consensusreference.NewConsensusReference(&tcAsConsensusPointer)).(*mempool)
mempool := New(mempoolConfig, consensusreference.NewConsensusReference(&tcAsConsensusPointer)).(*mempool)
res := mempool.IsTransactionOutputDust(&test.txOut)
if res != test.isDust {
@ -306,7 +306,7 @@ func TestCheckTransactionStandardInIsolation(t *testing.T) {
tcAsConsensus := tc.(externalapi.Consensus)
tcAsConsensusPointer := &tcAsConsensus
consensusReference := consensusreference.NewConsensusReference(&tcAsConsensusPointer)
mempool := New(mempoolConfig, tc.DAGParams(), consensusReference).(*mempool)
mempool := New(mempoolConfig, consensusReference).(*mempool)
// Ensure standardness is as expected.
err := mempool.checkTransactionStandardInIsolation(test.tx)

View File

@ -6,7 +6,7 @@ import (
"github.com/kaspanet/kaspad/domain/consensus/utils/transactionhelper"
)
func (mp *mempool) handleNewBlockTransactions(blockTransactions []*externalapi.DomainTransaction, clone bool) (
func (mp *mempool) handleNewBlockTransactions(blockTransactions []*externalapi.DomainTransaction,) (
[]*externalapi.DomainTransaction, error) {
// Skip the coinbase transaction
@ -30,7 +30,7 @@ func (mp *mempool) handleNewBlockTransactions(blockTransactions []*externalapi.D
return nil, err
}
acceptedOrphansFromThisTransaction, err := mp.orphansPool.processOrphansAfterAcceptedTransaction(transaction, clone)
acceptedOrphansFromThisTransaction, err := mp.orphansPool.processOrphansAfterAcceptedTransaction(transaction)
if err != nil {
return nil, err
}

View File

@ -4,7 +4,6 @@ import (
"sync"
"github.com/kaspanet/kaspad/domain/consensusreference"
"github.com/kaspanet/kaspad/domain/dagconfig"
"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
miningmanagermodel "github.com/kaspanet/kaspad/domain/miningmanager/model"
@ -14,7 +13,6 @@ type mempool struct {
mtx sync.RWMutex
config *Config
params *dagconfig.Params
consensusReference consensusreference.ConsensusReference
mempoolUTXOSet *mempoolUTXOSet
@ -23,10 +21,9 @@ type mempool struct {
}
// New constructs a new mempool
func New(config *Config, params *dagconfig.Params, consensusReference consensusreference.ConsensusReference) miningmanagermodel.Mempool {
func New(config *Config, consensusReference consensusreference.ConsensusReference) miningmanagermodel.Mempool {
mp := &mempool{
config: config,
params: params,
consensusReference: consensusReference,
}
@ -43,30 +40,31 @@ func (mp *mempool) ValidateAndInsertTransaction(transaction *externalapi.DomainT
mp.mtx.Lock()
defer mp.mtx.Unlock()
return mp.validateAndInsertTransaction(transaction, isHighPriority, allowOrphan, true)
return mp.validateAndInsertTransaction(transaction, isHighPriority, allowOrphan)
}
func (mp *mempool) GetTransaction(transactionID *externalapi.DomainTransactionID,
includeTransactionPool bool,
includeOrphanPool bool) (
transactionPoolTransaction *externalapi.DomainTransaction,
orphanPoolTransaction *externalapi.DomainTransaction,
transaction *externalapi.DomainTransaction,
isOrphan bool,
found bool) {
mp.mtx.RLock()
defer mp.mtx.RUnlock()
var transactionfound bool
var orphanTransactionFound bool
isOrphan = false
if includeTransactionPool {
transactionPoolTransaction, transactionfound = mp.transactionsPool.getTransaction(transactionID, true)
transaction, transactionfound = mp.transactionsPool.getTransaction(transactionID, true)
}
if includeOrphanPool {
orphanPoolTransaction, orphanTransactionFound = mp.orphansPool.getOrphanTransaction(transactionID, true)
if !transactionfound && includeOrphanPool {
transaction, transactionfound = mp.orphansPool.getOrphanTransaction(transactionID)
isOrphan = true
}
return transactionPoolTransaction, orphanPoolTransaction, transactionfound || orphanTransactionFound
return transaction, isOrphan, transactionfound
}
func (mp *mempool) GetTransactionsByAddresses(includeTransactionPool bool, includeOrphanPool bool) (
@ -79,14 +77,14 @@ func (mp *mempool) GetTransactionsByAddresses(includeTransactionPool bool, inclu
defer mp.mtx.RUnlock()
if includeTransactionPool {
sendingInTransactionPool, receivingInTransactionPool, err = mp.transactionsPool.getTransactionsByAddresses(true)
sendingInTransactionPool, receivingInTransactionPool, err = mp.transactionsPool.getTransactionsByAddresses()
if err != nil {
return nil, nil, nil, nil, err
}
}
if includeOrphanPool {
sendingInTransactionPool, receivingInOrphanPool, err = mp.orphansPool.getOrphanTransactionsByAddresses(true)
sendingInTransactionPool, receivingInOrphanPool, err = mp.orphansPool.getOrphanTransactionsByAddresses()
if err != nil {
return nil, nil, nil, nil, err
}
@ -103,11 +101,11 @@ func (mp *mempool) AllTransactions(includeTransactionPool bool, includeOrphanPoo
defer mp.mtx.RUnlock()
if includeTransactionPool {
transactionPoolTransactions = mp.transactionsPool.getAllTransactions(true)
transactionPoolTransactions = mp.transactionsPool.getAllTransactions()
}
if includeOrphanPool {
orphanPoolTransactions = mp.orphansPool.getAllOrphanTransactions(true)
orphanPoolTransactions = mp.orphansPool.getAllOrphanTransactions()
}
return transactionPoolTransactions, orphanPoolTransactions
@ -135,21 +133,21 @@ func (mp *mempool) HandleNewBlockTransactions(transactions []*externalapi.Domain
mp.mtx.Lock()
defer mp.mtx.Unlock()
return mp.handleNewBlockTransactions(transactions, true)
return mp.handleNewBlockTransactions(transactions)
}
func (mp *mempool) BlockCandidateTransactions() []*externalapi.DomainTransaction {
mp.mtx.RLock()
defer mp.mtx.RUnlock()
return mp.transactionsPool.allReadyTransactions(true)
return mp.transactionsPool.allReadyTransactions()
}
func (mp *mempool) RevalidateHighPriorityTransactions() (validTransactions []*externalapi.DomainTransaction, err error) {
mp.mtx.Lock()
defer mp.mtx.Unlock()
return mp.revalidateHighPriorityTransactions(true)
return mp.revalidateHighPriorityTransactions()
}
func (mp *mempool) RemoveTransactions(transactions []*externalapi.DomainTransaction, removeRedeemers bool) error {

View File

@ -134,7 +134,7 @@ func (op *orphansPool) addOrphan(transaction *externalapi.DomainTransaction, isH
return nil
}
func (op *orphansPool) processOrphansAfterAcceptedTransaction(acceptedTransaction *externalapi.DomainTransaction, clone bool) (
func (op *orphansPool) processOrphansAfterAcceptedTransaction(acceptedTransaction *externalapi.DomainTransaction) (
acceptedOrphans []*externalapi.DomainTransaction, err error) {
acceptedOrphans = []*externalapi.DomainTransaction{}
@ -169,11 +169,7 @@ func (op *orphansPool) processOrphansAfterAcceptedTransaction(acceptedTransactio
}
return nil, err
}
if clone {
acceptedOrphans = append(acceptedOrphans, orphan.Transaction().Clone())
} else {
acceptedOrphans = append(acceptedOrphans, orphan.Transaction())
}
acceptedOrphans = append(acceptedOrphans, orphan.Transaction().Clone()) //these pointers leave the mempool, hence the clone
}
}
}
@ -333,17 +329,14 @@ func (op *orphansPool) randomNonHighPriorityOrphan() *model.OrphanTransaction {
return nil
}
func (op *orphansPool) getOrphanTransaction(transactionID *externalapi.DomainTransactionID, clone bool) (*externalapi.DomainTransaction, bool) {
func (op *orphansPool) getOrphanTransaction(transactionID *externalapi.DomainTransactionID) (*externalapi.DomainTransaction, bool) {
if orphanTransaction, ok := op.allOrphans[*transactionID]; ok {
if clone {
return orphanTransaction.Transaction().Clone(), true
}
return orphanTransaction.Transaction(), true
return orphanTransaction.Transaction().Clone(), true //this pointer leaves the mempool, hence we clone.
}
return nil, false
}
func (op *orphansPool) getOrphanTransactionsByAddresses(clone bool) (
func (op *orphansPool) getOrphanTransactionsByAddresses() (
sending model.ScriptPublicKeyStringToDomainTransaction,
receiving model.ScriptPublicKeyStringToDomainTransaction,
err error) {
@ -351,11 +344,7 @@ func (op *orphansPool) getOrphanTransactionsByAddresses(clone bool) (
receiving = make(model.ScriptPublicKeyStringToDomainTransaction)
var transaction *externalapi.DomainTransaction
for _, mempoolTransaction := range op.allOrphans {
if clone {
transaction = mempoolTransaction.Transaction().Clone()
} else {
transaction = mempoolTransaction.Transaction()
}
transaction = mempoolTransaction.Transaction().Clone() //these pointers leave the mempool, hence we clone.
for _, input := range transaction.Inputs {
if input.UTXOEntry == nil { //this is not a bug, but a valid state of orphan transactions with missing outpoints.
continue
@ -371,15 +360,11 @@ func (op *orphansPool) getOrphanTransactionsByAddresses(clone bool) (
return sending, receiving, nil
}
func (op *orphansPool) getAllOrphanTransactions(clone bool) []*externalapi.DomainTransaction {
func (op *orphansPool) getAllOrphanTransactions() []*externalapi.DomainTransaction {
allOrphanTransactions := make([]*externalapi.DomainTransaction, len(op.allOrphans))
i := 0
for _, mempoolTransaction := range op.allOrphans {
if clone {
allOrphanTransactions[i] = mempoolTransaction.Transaction().Clone()
} else {
allOrphanTransactions[i] = mempoolTransaction.Transaction()
}
allOrphanTransactions[i] = mempoolTransaction.Transaction().Clone() //these pointers leave the mempool, hence we clone.
i++
}
return allOrphanTransactions

View File

@ -6,24 +6,11 @@ import (
"github.com/kaspanet/kaspad/infrastructure/logger"
)
func (mp *mempool) revalidateHighPriorityTransactions(clone bool) ([]*externalapi.DomainTransaction, error) {
func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTransaction, error) {
onEnd := logger.LogAndMeasureExecutionTime(log, "revalidateHighPriorityTransactions")
defer onEnd()
validTransactions := []*externalapi.DomainTransaction{}
if !clone {
for _, transaction := range mp.transactionsPool.highPriorityTransactions {
isValid, err := mp.revalidateTransaction(transaction)
if err != nil {
return nil, err
}
if !isValid {
continue
}
validTransactions = append(validTransactions, transaction.Transaction())
}
} else {
for _, transaction := range mp.transactionsPool.highPriorityTransactions {
isValid, err := mp.revalidateTransaction(transaction)
if err != nil {
@ -35,7 +22,6 @@ func (mp *mempool) revalidateHighPriorityTransactions(clone bool) ([]*externalap
validTransactions = append(validTransactions, transaction.Transaction().Clone())
}
}
return validTransactions, nil
}

View File

@ -131,16 +131,12 @@ func (tp *transactionsPool) expireOldTransactions() error {
return nil
}
func (tp *transactionsPool) allReadyTransactions(clone bool) []*externalapi.DomainTransaction {
func (tp *transactionsPool) allReadyTransactions() []*externalapi.DomainTransaction {
result := []*externalapi.DomainTransaction{}
for _, mempoolTransaction := range tp.allTransactions {
if len(mempoolTransaction.ParentTransactionsInPool()) == 0 {
if clone {
result = append(result, mempoolTransaction.Transaction().Clone())
} else {
result = append(result, mempoolTransaction.Transaction())
}
}
}
@ -219,7 +215,7 @@ func (tp *transactionsPool) getTransaction(transactionID *externalapi.DomainTran
return nil, false
}
func (tp *transactionsPool) getTransactionsByAddresses(clone bool) (
func (tp *transactionsPool) getTransactionsByAddresses() (
sending model.ScriptPublicKeyStringToDomainTransaction,
receiving model.ScriptPublicKeyStringToDomainTransaction,
err error) {
@ -227,13 +223,9 @@ func (tp *transactionsPool) getTransactionsByAddresses(clone bool) (
receiving = make(model.ScriptPublicKeyStringToDomainTransaction)
var transaction *externalapi.DomainTransaction
for _, mempoolTransaction := range tp.allTransactions {
if clone {
transaction = mempoolTransaction.Transaction().Clone()
} else {
transaction = mempoolTransaction.Transaction()
}
for _, input := range transaction.Inputs {
if input.UTXOEntry == nil { //this should be fixed
if input.UTXOEntry == nil {
return nil, nil, errors.Errorf("Mempool transaction %s is missing an UTXOEntry. This should be fixed, and not happen", consensushashing.TransactionID(transaction).String())
}
sending[input.UTXOEntry.ScriptPublicKey().String()] = transaction
@ -245,15 +237,11 @@ func (tp *transactionsPool) getTransactionsByAddresses(clone bool) (
return sending, receiving, nil
}
func (tp *transactionsPool) getAllTransactions(clone bool) []*externalapi.DomainTransaction {
func (tp *transactionsPool) getAllTransactions() []*externalapi.DomainTransaction {
allTransactions := make([]*externalapi.DomainTransaction, len(tp.allTransactions))
i := 0
for _, mempoolTransaction := range tp.allTransactions {
if clone {
allTransactions[i] = mempoolTransaction.Transaction().Clone()
} else {
allTransactions[i] = mempoolTransaction.Transaction()
}
i++
}
return allTransactions

View File

@ -10,7 +10,7 @@ import (
)
func (mp *mempool) validateAndInsertTransaction(transaction *externalapi.DomainTransaction, isHighPriority bool,
allowOrphan bool, clone bool) (acceptedTransactions []*externalapi.DomainTransaction, err error) {
allowOrphan bool) (acceptedTransactions []*externalapi.DomainTransaction, err error) {
onEnd := logger.LogAndMeasureExecutionTime(log,
fmt.Sprintf("validateAndInsertTransaction %s", consensushashing.TransactionID(transaction)))
@ -49,16 +49,12 @@ func (mp *mempool) validateAndInsertTransaction(transaction *externalapi.DomainT
return nil, err
}
acceptedOrphans, err := mp.orphansPool.processOrphansAfterAcceptedTransaction(mempoolTransaction.Transaction(), clone)
acceptedOrphans, err := mp.orphansPool.processOrphansAfterAcceptedTransaction(mempoolTransaction.Transaction(),)
if err != nil {
return nil, err
}
if clone {
acceptedTransactions = append([]*externalapi.DomainTransaction{transaction.Clone()}, acceptedOrphans...)
} else {
acceptedTransactions = append([]*externalapi.DomainTransaction{transaction}, acceptedOrphans...)
}
err = mp.transactionsPool.limitTransactionCount()
if err != nil {

View File

@ -17,7 +17,7 @@ type MiningManager interface {
GetBlockTemplateBuilder() miningmanagermodel.BlockTemplateBuilder
GetTransaction(transactionID *externalapi.DomainTransactionID, includeTransactionPool bool, includeOrphanPool bool) (
transactionPoolTransaction *externalapi.DomainTransaction,
orphanPoolTransaction *externalapi.DomainTransaction,
isOrphan bool,
found bool)
GetTransactionsByAddresses(includeTransactionPool bool, includeOrphanPool bool) (
sendingInTransactionPool map[string]*externalapi.DomainTransaction,
@ -122,7 +122,7 @@ func (mm *miningManager) GetTransaction(
includeTransactionPool bool,
includeOrphanPool bool) (
transactionPoolTransaction *externalapi.DomainTransaction,
orphanPoolTransaction *externalapi.DomainTransaction,
isOrphan bool,
found bool) {
return mm.mempool.GetTransaction(transactionID, includeTransactionPool, includeOrphanPool)

View File

@ -227,7 +227,6 @@ func TestHandleNewBlockTransactions(t *testing.T) {
}
mempoolTransactions, _ = miningManager.AllTransactions(true, false)
if len(mempoolTransactions) != 0 {
mempoolTransactions, _ = miningManager.AllTransactions(true, false)
blockIDs := domainBlocksToBlockIds(mempoolTransactions)
t.Fatalf("The mempool contains unexpected transactions: %s", blockIDs)
}
@ -608,7 +607,7 @@ func TestModifyBlockTemplate(t *testing.T) {
t.Fatalf("ValidateAndInsertTransaction: %v", err)
}
}
transactionsMempool, _ := miningManager.AllTransactions(true, true)
transactionsMempool, _ := miningManager.AllTransactions(true, false)
for _, transaction := range transactionsMempool {
if contains(transaction, childTransactions) {
t.Fatalf("Error: an orphan transaction is exist in the mempool")
@ -657,7 +656,7 @@ func TestModifyBlockTemplate(t *testing.T) {
if err != nil {
t.Fatalf("HandleNewBlockTransactions: %+v", err)
}
transactionsMempool, _ = miningManager.AllTransactions(true, true)
transactionsMempool, _ = miningManager.AllTransactions(true, false)
if len(transactionsMempool) != len(childTransactions) {
t.Fatalf("Expected %d transactions in the mempool but got %d", len(childTransactions), len(transactionsMempool))
}

View File

@ -18,7 +18,7 @@ type Mempool interface {
includeOrphanPool bool,
) (
transactionPoolTransaction *externalapi.DomainTransaction,
orphanPoolTransaction *externalapi.DomainTransaction,
isOrphan bool,
found bool)
GetTransactionsByAddresses(
includeTransactionPool bool,

View File

@ -103,7 +103,6 @@ func TestTxRelay(t *testing.T) {
t.Fatal("Error payer is reciving")
}
}
for _, mempoolEntry := range mempoolEntryByAddress.Receiving {
if mempoolEntry.IsOrphan {
t.Fatalf("transaction %s is an orphan, although it shouldn't be", mempoolEntry.Transaction.VerboseData.TransactionID)