From 248ce9f1fc79d8f7168548a5653084daccd6d0ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Pansier?= Date: Fri, 27 Jan 2023 16:08:57 +0100 Subject: [PATCH] fix(node): transactions can be older than 1 block (#189) --- .../protocol/validation/transactions_pool.go | 30 ++++---- src/node/protocol/verification/blockchain.go | 6 +- .../protocol/protocoltest/block_factory.go | 2 +- .../validation/transactions_pool_test.go | 7 +- .../protocol/verification/blockchain_test.go | 77 +++++++++++-------- 5 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/node/protocol/validation/transactions_pool.go b/src/node/protocol/validation/transactions_pool.go index faada81a..03a874d9 100644 --- a/src/node/protocol/validation/transactions_pool.go +++ b/src/node/protocol/validation/transactions_pool.go @@ -74,18 +74,16 @@ func (pool *TransactionsPool) Validate(timestamp int64) { continue } if len(blocks) > 1 { - if transaction.Timestamp < blocks[len(blocks)-2].Timestamp { + if transaction.Timestamp < blocks[len(blocks)-1].Timestamp { pool.logger.Warn(fmt.Sprintf("transaction removed from the transactions pool, the transaction timestamp is too old, transaction: %v", transaction)) rejectedTransactions = append(rejectedTransactions, transaction) continue } - for j := len(blocks) - 2; j < len(blocks); j++ { - for _, validatedTransaction := range blocks[j].Transactions { - if pool.transactions[i].Equals(validatedTransaction) { - pool.logger.Warn(fmt.Sprintf("transaction removed from the transactions pool, the transaction is already in the blockchain, transaction: %v", transaction)) - rejectedTransactions = append(rejectedTransactions, transaction) - continue - } + for _, validatedTransaction := range blocks[len(blocks)-1].Transactions { + if pool.transactions[i].Equals(validatedTransaction) { + pool.logger.Warn(fmt.Sprintf("transaction removed from the transactions pool, the transaction is already in the blockchain, transaction: %v", transaction)) + rejectedTransactions = append(rejectedTransactions, transaction) + continue } } } @@ -170,17 +168,15 @@ func (pool *TransactionsPool) addTransaction(transactionRequest *network.Transac err = fmt.Errorf("the transaction timestamp is too far in the future: %v, now: %v", time.Unix(0, timestamp), time.Unix(0, nextBlockTimestamp)) return } - previousBlockTimestamp := blocks[len(blocks)-2].Timestamp - if timestamp < previousBlockTimestamp { - err = fmt.Errorf("the transaction timestamp is too old: %v, previous block timestamp: %v", time.Unix(0, timestamp), time.Unix(0, previousBlockTimestamp)) + currentBlockTimestamp := blocks[len(blocks)-1].Timestamp + if timestamp < currentBlockTimestamp { + err = fmt.Errorf("the transaction timestamp is too old: %v, current block timestamp: %v", time.Unix(0, timestamp), time.Unix(0, currentBlockTimestamp)) return } - for i := len(blocks) - 2; i < len(blocks); i++ { - for _, validatedTransaction := range blocks[i].Transactions { - if transaction.Equals(validatedTransaction) { - err = errors.New("the transaction is already in the blockchain") - return - } + for _, validatedTransaction := range blocks[len(blocks)-1].Transactions { + if transaction.Equals(validatedTransaction) { + err = errors.New("the transaction is already in the blockchain") + return } } } diff --git a/src/node/protocol/verification/blockchain.go b/src/node/protocol/verification/blockchain.go index 445e27ab..895b4eda 100644 --- a/src/node/protocol/verification/blockchain.go +++ b/src/node/protocol/verification/blockchain.go @@ -472,10 +472,10 @@ func (blockchain *Blockchain) verify(neighborBlocks []*network.BlockResponse, ho totalTransactionsValueBySenderAddress[transaction.SenderAddress()] += transaction.Value() + fee totalTransactionsFees += fee if currentBlockTimestamp+blockchain.validationTimer.Nanoseconds() < transaction.Timestamp() { - return nil, fmt.Errorf("a neighbor block transaction timestamp is too far in the future, transaction: %v", transaction) + return nil, fmt.Errorf("a neighbor block transaction timestamp is too far in the future, transaction: %v", transaction.GetResponse()) } - if i > 0 && transaction.Timestamp() < neighborBlocks[i-1].Timestamp { - return nil, fmt.Errorf("a neighbor block transaction timestamp is too old, transaction: %v", transaction) + if i > 0 && transaction.Timestamp() < neighborBlocks[i].Timestamp { + return nil, fmt.Errorf("a neighbor block transaction timestamp is too old, transaction: %v", transaction.GetResponse()) } } } diff --git a/test/node/protocol/protocoltest/block_factory.go b/test/node/protocol/protocoltest/block_factory.go index 6591a11f..7deef0bb 100644 --- a/test/node/protocol/protocoltest/block_factory.go +++ b/test/node/protocol/protocoltest/block_factory.go @@ -11,7 +11,7 @@ func NewGenesisBlockResponse(validatorWalletAddress string) *network.BlockRespon Timestamp: 0, PreviousHash: [32]byte{}, Transactions: []*network.TransactionResponse{genesisTransaction}, - RegisteredAddresses: nil, + RegisteredAddresses: []string{validatorWalletAddress}, } } diff --git a/test/node/protocol/validation/transactions_pool_test.go b/test/node/protocol/validation/transactions_pool_test.go index f786a039..298c624f 100644 --- a/test/node/protocol/validation/transactions_pool_test.go +++ b/test/node/protocol/validation/transactions_pool_test.go @@ -48,23 +48,22 @@ func Test_AddTransaction_TransactionTimestampIsInTheFuture_TransactionNotAdded(t test.Assert(t, actualTransactionsLength == expectedTransactionsLength, fmt.Sprintf("Wrong transactions count. Expected: %d - Actual: %d", expectedTransactionsLength, actualTransactionsLength)) } -func Test_AddTransaction_TransactionTimestampIsOlderThan2Blocks_TransactionNotAdded(t *testing.T) { +func Test_AddTransaction_TransactionTimestampIsOlderThan1Blocks_TransactionNotAdded(t *testing.T) { // Arrange validatorWallet, _ := encryption.DecodeWallet(test.Mnemonic1, test.DerivationPath, "", "") validatorWalletAddress := validatorWallet.Address() registryMock := new(protocoltest.RegistryMock) registryMock.IsRegisteredFunc = func(string) (bool, error) { return true, nil } watchMock := new(clocktest.WatchMock) - var now int64 = 3 + var now int64 = 2 watchMock.NowFunc = func() time.Time { return time.Unix(0, now) } validationTimer := time.Nanosecond logger := logtest.NewLoggerMock() - invalidTransaction := server.NewTransaction("A", validatorWalletAddress, validatorWallet.PublicKey(), now-3, 1) + invalidTransaction := server.NewTransaction("A", validatorWalletAddress, validatorWallet.PublicKey(), now-2, 1) _ = invalidTransaction.Sign(validatorWallet.PrivateKey()) invalidTransactionRequest := invalidTransaction.GetRequest() var blockResponses []*network.BlockResponse blockResponses = append(blockResponses, protocoltest.NewGenesisBlockResponse(validatorWalletAddress)) - blockResponses = append(blockResponses, protocoltest.NewEmptyBlockResponse(now-2)) blockResponses = append(blockResponses, protocoltest.NewEmptyBlockResponse(now-1)) blockchainMock := new(protocoltest.BlockchainMock) blockchainMock.BlocksFunc = func() []*network.BlockResponse { return blockResponses } diff --git a/test/node/protocol/verification/blockchain_test.go b/test/node/protocol/verification/blockchain_test.go index df6487c6..aad35647 100644 --- a/test/node/protocol/verification/blockchain_test.go +++ b/test/node/protocol/verification/blockchain_test.go @@ -1,6 +1,7 @@ package verification import ( + "fmt" "github.com/my-cloud/ruthenium/src/encryption" "github.com/my-cloud/ruthenium/src/node/network" "github.com/my-cloud/ruthenium/src/node/protocol/validation" @@ -11,6 +12,7 @@ import ( "github.com/my-cloud/ruthenium/test/node/clock/clocktest" "github.com/my-cloud/ruthenium/test/node/network/networktest" "github.com/my-cloud/ruthenium/test/node/protocol/protocoltest" + "strings" "testing" "time" ) @@ -62,7 +64,7 @@ func Test_CalculateTotalAmount_InitialValidator_ReturnsGenesisAmount(t *testing. amount := blockchain.CalculateTotalAmount(1, genesisTransaction.RecipientAddress) // Assert - test.Assert(t, amount == genesisTransaction.Value, "calculated amount should be the genesis amount") + test.Assert(t, amount == genesisTransaction.Value, "calculated amount is not the genesis amount whereas it should be") } func Test_Update_NeighborBlockchainIsBetter_IsReplaced(t *testing.T) { @@ -174,14 +176,17 @@ func Test_Update_NeighborNewBlockTimestampIsInvalid_IsNotReplaced(t *testing.T) // Assert var isKept bool + var isExplicitMessageLogged bool for _, call := range logger.DebugCalls() { + expectedMessage := "neighbor block timestamp is invalid" if call.Msg == blockchainKeptMessage { isKept = true + } else if strings.Contains(call.Msg, expectedMessage) { + isExplicitMessageLogged = true } } - if !isKept { - t.Errorf("blockchain is replaced whereas it should be kept") - } + test.Assert(t, isKept, "blockchain is replaced whereas it should be kept") + test.Assert(t, isExplicitMessageLogged, "no explicit message is logged whereas it should be") }) } } @@ -216,12 +221,17 @@ func Test_Update_NeighborNewBlockTimestampIsInTheFuture_IsNotReplaced(t *testing // Assert var isKept bool + var isExplicitMessageLogged bool for _, call := range logger.DebugCalls() { + expectedMessage := "neighbor block timestamp is in the future" if call.Msg == blockchainKeptMessage { isKept = true + } else if strings.Contains(call.Msg, expectedMessage) { + isExplicitMessageLogged = true } } test.Assert(t, isKept, "blockchain is replaced whereas it should be kept") + test.Assert(t, isExplicitMessageLogged, "no explicit message is logged whereas it should be") } func Test_Update_NeighborNewBlockTransactionTimestampIsTooFarInTheFuture_IsNotReplaced(t *testing.T) { @@ -232,23 +242,21 @@ func Test_Update_NeighborNewBlockTransactionTimestampIsTooFarInTheFuture_IsNotRe watchMock.NowFunc = func() time.Time { return time.Unix(0, 1) } logger := logtest.NewLoggerMock() neighborMock := new(networktest.NeighborMock) + wallet, _ := encryption.DecodeWallet(test.Mnemonic1, test.DerivationPath, "", "") + address := wallet.Address() + serverTransaction := server.NewTransaction("A", address, wallet.PublicKey(), 3, 1) + _ = serverTransaction.Sign(wallet.PrivateKey()) + transactionRequest := serverTransaction.GetRequest() + transaction, _ := validation.NewTransactionFromRequest(&transactionRequest) + transactionResponse := transaction.GetResponse() neighborMock.GetBlocksFunc = func() ([]*network.BlockResponse, error) { - wallet, _ := encryption.DecodeWallet(test.Mnemonic1, test.DerivationPath, "", "") - address := wallet.Address() blockResponse1 := protocoltest.NewGenesisBlockResponse(address) block1, _ := verification.NewBlockFromResponse(blockResponse1) hash, _ := block1.Hash() var block2Timestamp int64 = 1 - serverTransaction := server.NewTransaction("A", wallet.Address(), wallet.PublicKey(), 3, 1) - _ = serverTransaction.Sign(wallet.PrivateKey()) transactions := []*network.TransactionResponse{ - { - RecipientAddress: address, - SenderAddress: "REWARD_SENDER_ADDRESS", - Timestamp: block2Timestamp, - Value: 0, - Fee: 0, - }, + transactionResponse, + validation.NewRewardTransaction(address, block2Timestamp, 0), } var registeredAddresses []string registeredAddresses = append(registeredAddresses, address) @@ -270,12 +278,17 @@ func Test_Update_NeighborNewBlockTransactionTimestampIsTooFarInTheFuture_IsNotRe // Assert var isKept bool + var isExplicitMessageLogged bool for _, call := range logger.DebugCalls() { + expectedMessage := fmt.Sprintf("a neighbor block transaction timestamp is too far in the future, transaction: %v", transactionResponse) if call.Msg == blockchainKeptMessage { isKept = true + } else if strings.Contains(call.Msg, expectedMessage) { + isExplicitMessageLogged = true } } test.Assert(t, isKept, "blockchain is replaced whereas it should be kept") + test.Assert(t, isExplicitMessageLogged, "no explicit message is logged whereas it should be") } func Test_Update_NeighborNewBlockTransactionTimestampIsTooOld_IsNotReplaced(t *testing.T) { @@ -286,31 +299,26 @@ func Test_Update_NeighborNewBlockTransactionTimestampIsTooOld_IsNotReplaced(t *t watchMock.NowFunc = func() time.Time { return time.Unix(0, 2) } logger := logtest.NewLoggerMock() neighborMock := new(networktest.NeighborMock) + wallet, _ := encryption.DecodeWallet(test.Mnemonic1, test.DerivationPath, "", "") + address := wallet.Address() + serverTransaction := server.NewTransaction("A", address, wallet.PublicKey(), 0, 1) + _ = serverTransaction.Sign(wallet.PrivateKey()) + transactionRequest := serverTransaction.GetRequest() + transaction, _ := validation.NewTransactionFromRequest(&transactionRequest) + transactionResponse := transaction.GetResponse() neighborMock.GetBlocksFunc = func() ([]*network.BlockResponse, error) { - wallet, _ := encryption.DecodeWallet(test.Mnemonic1, test.DerivationPath, "", "") - address := wallet.Address() blockResponse1 := protocoltest.NewGenesisBlockResponse(address) block1, _ := verification.NewBlockFromResponse(blockResponse1) hash1, _ := block1.Hash() - blockResponse2 := protocoltest.NewRewardedBlockResponse(hash1, 1) - block2, _ := verification.NewBlockFromResponse(blockResponse2) - hash2, _ := block2.Hash() - var block3Timestamp int64 = 2 - serverTransaction := server.NewTransaction("A", wallet.Address(), wallet.PublicKey(), 0, 1) - _ = serverTransaction.Sign(wallet.PrivateKey()) + var block2Timestamp int64 = 1 transactions := []*network.TransactionResponse{ - { - RecipientAddress: address, - SenderAddress: "REWARD_SENDER_ADDRESS", - Timestamp: block3Timestamp, - Value: 0, - Fee: 0, - }, + transactionResponse, + validation.NewRewardTransaction(address, block2Timestamp, 0), } var registeredAddresses []string registeredAddresses = append(registeredAddresses, address) - blockResponse3 := verification.NewBlockResponse(block3Timestamp, hash2, transactions, registeredAddresses) - return []*network.BlockResponse{blockResponse1, blockResponse2, blockResponse3}, nil + blockResponse2 := verification.NewBlockResponse(block2Timestamp, hash1, transactions, registeredAddresses) + return []*network.BlockResponse{blockResponse1, blockResponse2}, nil } neighborMock.TargetFunc = func() string { return "neighbor" @@ -327,10 +335,15 @@ func Test_Update_NeighborNewBlockTransactionTimestampIsTooOld_IsNotReplaced(t *t // Assert var isKept bool + var isExplicitMessageLogged bool for _, call := range logger.DebugCalls() { + expectedMessage := fmt.Sprintf("a neighbor block transaction timestamp is too old, transaction: %v", transactionResponse) if call.Msg == blockchainKeptMessage { isKept = true + } else if strings.Contains(call.Msg, expectedMessage) { + isExplicitMessageLogged = true } } test.Assert(t, isKept, "blockchain is replaced whereas it should be kept") + test.Assert(t, isExplicitMessageLogged, "no explicit message is logged whereas it should be") }