From f109cb1ee910eaf631cb643df87fda4d2829e7c0 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 14 Mar 2024 16:28:54 +0000 Subject: [PATCH] Remove usages of frontiers table from ledger_processor block checks (#4460) The account can be derived from the previous block's account field. --- nano/core_test/block_store.cpp | 5 ----- nano/core_test/ledger.cpp | 22 ++-------------------- nano/secure/ledger.cpp | 28 ++++++++++++---------------- 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 07ca6d9b08..ecd575ea41 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -820,11 +820,6 @@ TEST (block_store, frontier) auto transaction (store->tx_begin_write ()); nano::block_hash hash (100); nano::account account (200); - ASSERT_TRUE (store->frontier.get (transaction, hash).is_zero ()); - store->frontier.put (transaction, hash, account); - ASSERT_EQ (account, store->frontier.get (transaction, hash)); - store->frontier.del (transaction, hash); - ASSERT_TRUE (store->frontier.get (transaction, hash).is_zero ()); } TEST (block_store, block_replace) diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index f74d115a62..01c844728b 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -103,16 +103,13 @@ TEST (ledger, process_send) .work (*pool.generate (info1->head)) .build (); nano::block_hash hash1 = send->hash (); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, info1->head)); ASSERT_EQ (1, info1->block_count); // This was a valid block, it should progress. auto return1 = ledger.process (transaction, send); + ASSERT_EQ (nano::block_status::progress, return1); ASSERT_EQ (nano::dev::genesis_key.pub, send->sideband ().account); ASSERT_EQ (2, send->sideband ().height); ASSERT_EQ (nano::dev::constants.genesis_amount - 50, ledger.amount (transaction, hash1)); - ASSERT_TRUE (store.frontier.get (transaction, info1->head).is_zero ()); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, hash1)); - ASSERT_EQ (nano::block_status::progress, return1); ASSERT_EQ (nano::dev::genesis_key.pub, send->account ()); ASSERT_EQ (50, ledger.account_balance (transaction, nano::dev::genesis_key.pub)); ASSERT_EQ (nano::dev::constants.genesis_amount - 50, ledger.account_receivable (transaction, key2.pub)); @@ -144,7 +141,6 @@ TEST (ledger, process_send) ASSERT_EQ (nano::block_status::progress, return2); ASSERT_EQ (key2.pub, open->account ()); ASSERT_EQ (nano::dev::constants.genesis_amount - 50, ledger.amount (transaction, hash2)); - ASSERT_EQ (key2.pub, store.frontier.get (transaction, hash2)); ASSERT_EQ (nano::dev::constants.genesis_amount - 50, ledger.account_balance (transaction, key2.pub)); ASSERT_EQ (0, ledger.account_receivable (transaction, key2.pub)); ASSERT_EQ (50, ledger.weight (nano::dev::genesis_key.pub)); @@ -164,7 +160,6 @@ TEST (ledger, process_send) ASSERT_NE (nullptr, latest5); ASSERT_EQ (*open, *latest5); ASSERT_FALSE (ledger.rollback (transaction, hash2)); - ASSERT_TRUE (store.frontier.get (transaction, hash2).is_zero ()); auto info5 = ledger.account_info (transaction, key2.pub); ASSERT_FALSE (info5); auto pending1 = ledger.pending_info (transaction, nano::pending_key (key2.pub, hash1)); @@ -181,8 +176,6 @@ TEST (ledger, process_send) ASSERT_EQ (hash1, info6->head); ASSERT_FALSE (ledger.rollback (transaction, info6->head)); ASSERT_EQ (nano::dev::constants.genesis_amount, ledger.weight (nano::dev::genesis_key.pub)); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, info1->head)); - ASSERT_TRUE (store.frontier.get (transaction, hash1).is_zero ()); auto info7 = ledger.account_info (transaction, nano::dev::genesis_key.pub); ASSERT_TRUE (info7); ASSERT_EQ (1, info7->block_count); @@ -250,14 +243,11 @@ TEST (ledger, process_receive) .work (*pool.generate (hash2)) .build (); auto hash4 = receive->hash (); - ASSERT_EQ (key2.pub, store.frontier.get (transaction, hash2)); auto return2 = ledger.process (transaction, receive); ASSERT_EQ (key2.pub, receive->sideband ().account); ASSERT_EQ (nano::dev::constants.genesis_amount - 25, receive->sideband ().balance.number ()); ASSERT_EQ (2, receive->sideband ().height); ASSERT_EQ (25, ledger.amount (transaction, hash4)); - ASSERT_TRUE (store.frontier.get (transaction, hash2).is_zero ()); - ASSERT_EQ (key2.pub, store.frontier.get (transaction, hash4)); ASSERT_EQ (nano::block_status::progress, return2); ASSERT_EQ (key2.pub, receive->account ()); ASSERT_EQ (hash4, ledger.latest (transaction, key2.pub)); @@ -267,8 +257,6 @@ TEST (ledger, process_receive) ASSERT_EQ (nano::dev::constants.genesis_amount - 25, ledger.weight (key3.pub)); ASSERT_FALSE (ledger.rollback (transaction, hash4)); ASSERT_TRUE (store.block.successor (transaction, hash2).is_zero ()); - ASSERT_EQ (key2.pub, store.frontier.get (transaction, hash2)); - ASSERT_TRUE (store.frontier.get (transaction, hash4).is_zero ()); ASSERT_EQ (25, ledger.account_balance (transaction, nano::dev::genesis_key.pub)); ASSERT_EQ (25, ledger.account_receivable (transaction, key2.pub)); ASSERT_EQ (nano::dev::constants.genesis_amount - 50, ledger.account_balance (transaction, key2.pub)); @@ -520,12 +508,9 @@ TEST (ledger, representative_change) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*pool.generate (info1->head)) .build (); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, info1->head)); auto return1 (ledger.process (transaction, block)); - ASSERT_EQ (0, ledger.amount (transaction, block->hash ())); - ASSERT_TRUE (store.frontier.get (transaction, info1->head).is_zero ()); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, block->hash ())); ASSERT_EQ (nano::block_status::progress, return1); + ASSERT_EQ (0, ledger.amount (transaction, block->hash ())); ASSERT_EQ (nano::dev::genesis_key.pub, block->account ()); ASSERT_EQ (0, ledger.weight (nano::dev::genesis_key.pub)); ASSERT_EQ (nano::dev::constants.genesis_amount, ledger.weight (key2.pub)); @@ -533,8 +518,6 @@ TEST (ledger, representative_change) ASSERT_TRUE (info2); ASSERT_EQ (block->hash (), info2->head); ASSERT_FALSE (ledger.rollback (transaction, info2->head)); - ASSERT_EQ (nano::dev::genesis_key.pub, store.frontier.get (transaction, info1->head)); - ASSERT_TRUE (store.frontier.get (transaction, block->hash ()).is_zero ()); auto info3 = ledger.account_info (transaction, nano::dev::genesis_key.pub); ASSERT_TRUE (info3); ASSERT_EQ (info1->head, info3->head); @@ -5496,7 +5479,6 @@ TEST (ledger, migrate_lmdb_to_rocksdb) ASSERT_EQ (*send, *block1); ASSERT_TRUE (rocksdb_store.peer.exists (rocksdb_transaction, endpoint_key)); ASSERT_EQ (rocksdb_store.version.get (rocksdb_transaction), version); - ASSERT_EQ (rocksdb_store.frontier.get (rocksdb_transaction, 2), 5); nano::confirmation_height_info confirmation_height_info; ASSERT_FALSE (rocksdb_store.confirmation_height.get (rocksdb_transaction, nano::dev::genesis_key.pub, confirmation_height_info)); ASSERT_EQ (confirmation_height_info.height, 2); diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index 8e69bde8ef..9472af3298 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -464,12 +464,12 @@ void ledger_processor::change_block (nano::change_block & block_a) result = block_a.valid_predecessor (*previous) ? nano::block_status::progress : nano::block_status::block_position; if (result == nano::block_status::progress) { - auto account (ledger.store.frontier.get (transaction, block_a.hashables.previous)); - result = account.is_zero () ? nano::block_status::fork : nano::block_status::progress; + auto account = previous->account (); + auto info = ledger.account_info (transaction, account); + debug_assert (info); + result = info->head != block_a.hashables.previous ? nano::block_status::fork : nano::block_status::progress; if (result == nano::block_status::progress) { - auto info = ledger.account_info (transaction, account); - debug_assert (info); debug_assert (info->head == block_a.hashables.previous); result = validate_message (account, hash, block_a.signature) ? nano::block_status::bad_signature : nano::block_status::progress; // Is this block signed correctly (Malformed) if (result == nano::block_status::progress) @@ -510,8 +510,10 @@ void ledger_processor::send_block (nano::send_block & block_a) result = block_a.valid_predecessor (*previous) ? nano::block_status::progress : nano::block_status::block_position; if (result == nano::block_status::progress) { - auto account (ledger.store.frontier.get (transaction, block_a.hashables.previous)); - result = account.is_zero () ? nano::block_status::fork : nano::block_status::progress; + auto account = previous->account (); + auto info = ledger.account_info (transaction, account); + debug_assert (info); + result = info->head != block_a.hashables.previous ? nano::block_status::fork : nano::block_status::progress; if (result == nano::block_status::progress) { result = validate_message (account, hash, block_a.signature) ? nano::block_status::bad_signature : nano::block_status::progress; // Is this block signed correctly (Malformed) @@ -522,8 +524,6 @@ void ledger_processor::send_block (nano::send_block & block_a) if (result == nano::block_status::progress) { debug_assert (!validate_message (account, hash, block_a.signature)); - auto info = ledger.account_info (transaction, account); - debug_assert (info); debug_assert (info->head == block_a.hashables.previous); result = info->balance.number () >= block_a.hashables.balance.number () ? nano::block_status::progress : nano::block_status::negative_spend; // Is this trying to spend a negative amount (Malicious) if (result == nano::block_status::progress) @@ -561,8 +561,10 @@ void ledger_processor::receive_block (nano::receive_block & block_a) result = block_a.valid_predecessor (*previous) ? nano::block_status::progress : nano::block_status::block_position; if (result == nano::block_status::progress) { - auto account (ledger.store.frontier.get (transaction, block_a.hashables.previous)); - result = account.is_zero () ? nano::block_status::gap_previous : nano::block_status::progress; // Have we seen the previous block? No entries for account at all (Harmless) + auto account = previous->account (); + auto info = ledger.account_info (transaction, account); + debug_assert (info); + result = info->head != block_a.hashables.previous ? nano::block_status::fork : nano::block_status::progress; // If we have the block but it's not the latest we have a signed fork (Malicious) if (result == nano::block_status::progress) { result = validate_message (account, hash, block_a.signature) ? nano::block_status::bad_signature : nano::block_status::progress; // Is the signature valid (Malformed) @@ -572,8 +574,6 @@ void ledger_processor::receive_block (nano::receive_block & block_a) result = ledger.block_or_pruned_exists (transaction, block_a.hashables.source) ? nano::block_status::progress : nano::block_status::gap_source; // Have we seen the source block already? (Harmless) if (result == nano::block_status::progress) { - auto info = ledger.account_info (transaction, account); - debug_assert (info); result = info->head == block_a.hashables.previous ? nano::block_status::progress : nano::block_status::gap_previous; // Block doesn't immediately follow latest block (Harmless) if (result == nano::block_status::progress) { @@ -613,10 +613,6 @@ void ledger_processor::receive_block (nano::receive_block & block_a) } } } - else - { - result = ledger.store.block.exists (transaction, block_a.hashables.previous) ? nano::block_status::fork : nano::block_status::gap_previous; // If we have the block but it's not the latest we have a signed fork (Malicious) - } } } }