fix(validators): check input metadata matches spending output #3761

sdbondi · 2022-01-27T10:05:36Z

Description

Add validation to check input metadata matches utxo to spend
Add more tests for consensus de/encoding and hash writer
Remove some unused protobuf types (MmrTree)
Add bad block check to add_block header validator
Unit tests for validation of metadata mismatch for async validator (already working) and body only validator (previously not working)

Motivation and Context

Find and reject blocks/txns where the metadata mismatches the UTXO
Ensure correctness of consensus encoding impl

How Has This Been Tested?

New unit tests, manually by testing the invalid faucet utxo is rejected

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs

base_layer/core/src/chain_storage/tests/blockchain_database.rs

SWvheerden

LGTM
I just have a NIT about some of the logs

SWvheerden · 2022-01-28T11:59:15Z

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -374,6 +374,8 @@ where B: BlockchainBackend + 'static
                        .await?,
                    self.consensus_manager.get_block_reward_at(height),
                );
+
+                debug!(target: LOG_TARGET, "New template block: {}", block_template);


Suggested change

debug!(target: LOG_TARGET, "New template block: {}", block_template);

trace!(target: LOG_TARGET, "New template block: {}", block_template);

this can be a very large log

Fair enough, I'll leave that for another PR so that we dont have to rerun tests

SWvheerden · 2022-01-28T11:59:30Z

applications/tari_base_node/src/grpc/base_node_grpc_server.rs

@@ -716,7 +716,8 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
        let block = Block::try_from(request)
            .map_err(|e| Status::invalid_argument(format!("Failed to convert arguments. Invalid block: {:?}", e)))?;
        let block_height = block.header.height;
-        debug!(
+        debug!(target: LOG_TARGET, "Miner submitted block: {}", block);


Suggested change

debug!(target: LOG_TARGET, "Miner submitted block: {}", block);

trace!(target: LOG_TARGET, "Miner submitted block: {}", block);

This will be a very large log

SWvheerden · 2022-01-28T11:59:44Z

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -384,6 +386,7 @@ where B: BlockchainBackend + 'static
                Ok(NodeCommsResponse::NewBlockTemplate(block_template))
            },
            NodeCommsRequest::GetNewBlock(block_template) => {
+                debug!(target: LOG_TARGET, "Prepared block: {}", block_template);


Suggested change

debug!(target: LOG_TARGET, "Prepared block: {}", block_template);

trace!(target: LOG_TARGET, "Prepared block: {}", block_template);

SWvheerden · 2022-01-28T12:00:11Z

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -675,6 +679,7 @@ where B: BlockchainBackend + 'static
                .map(|p| format!("remote peer: {}", p))
                .unwrap_or_else(|| "local services".to_string())
        );
+        debug!(target: LOG_TARGET, "Incoming block: {}", block);


Suggested change

debug!(target: LOG_TARGET, "Incoming block: {}", block);

trace!(target: LOG_TARGET, "Incoming block: {}", block);

stringhandler · 2022-01-28T19:24:54Z

Debug is fine. Info would be a problem

* development: (29 commits) feat(base-node): add base node prometheus metrics (tari-project#3773) fix(ffi): mut pointers should be const (tari-project#3775) chore: update launchpad for dibbler (tari-project#3769) v0.27.3 fix(ffi): missing param in header.h (tari-project#3774) v0.27.2 v0.27.2 fix(ffi): fix bad access (tari-project#3772) chore(collectibles): fix eslint v0.27.1 fix(ffi): fix null pointer assignment in import_utxo (tari-project#3770) fix(validators): check input metadata matches spending output (tari-project#3761) v0.27.0 feat(ffi)!: Add commitment_signature_create and destroy (tari-project#3768) feat(ffi)!: add features, metadata_signature and sender_offset_public_key to import_utxo (tari-project#3767) feat(explorer): better view on mempool (tari-project#3763) feat: update the available balance in console wallet (tari-project#3760) chore: reclassify very long log message (tari-project#3752) fix: fix cucumber test for standard recovery (tari-project#3757) style: format with prettier (same settings as in integration tests) (tari-project#3756) ...

sdbondi force-pushed the core-hash-fix branch from a05d6e1 to 04c200f Compare January 27, 2022 10:06

sdbondi changed the title ~~tests(core): add unit tests for all consensus encoding impls~~ test(core): add unit tests for all consensus encoding impls Jan 27, 2022

sdbondi force-pushed the core-hash-fix branch 3 times, most recently from 08a9ea5 to 494d966 Compare January 27, 2022 10:43

tests(core): add unit tests for all consensus encoding impls

6e25b10

sdbondi force-pushed the core-hash-fix branch from 494d966 to 6e25b10 Compare January 27, 2022 10:53

delta1 reviewed Jan 27, 2022

View reviewed changes

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs Outdated Show resolved Hide resolved

Remove fetch_mmr_size panic

b813c14

delta1 reviewed Jan 27, 2022

View reviewed changes

base_layer/core/src/chain_storage/tests/blockchain_database.rs Outdated Show resolved Hide resolved

delta1 previously approved these changes Jan 27, 2022

View reviewed changes

sdbondi dismissed delta1’s stale review via 2357540 January 27, 2022 18:19

sdbondi force-pushed the core-hash-fix branch from facdade to 2357540 Compare January 27, 2022 18:19

fix search-utxo command

ca517b7

sdbondi force-pushed the core-hash-fix branch from 2357540 to ca517b7 Compare January 28, 2022 04:34

fix(validators): check input metadata matches spending output

36db609

sdbondi changed the title ~~test(core): add unit tests for all consensus encoding impls~~ fix(core): validate input metadata matches spent utxo Jan 28, 2022

sdbondi changed the title ~~fix(core): validate input metadata matches spent utxo~~ fix(validators): check input metadata matches spending output Jan 28, 2022

sdbondi force-pushed the core-hash-fix branch from 57048d2 to 25c30f2 Compare January 28, 2022 11:28

stringhandler previously approved these changes Jan 28, 2022

View reviewed changes

Merge branch 'development' into core-hash-fix

8e3d4ac

sdbondi dismissed stringhandler’s stale review via 8e3d4ac January 28, 2022 11:35

sdbondi force-pushed the core-hash-fix branch from 25c30f2 to 8e3d4ac Compare January 28, 2022 11:35

stringhandler added the mq-approved label Jan 28, 2022

stringhandler approved these changes Jan 28, 2022

View reviewed changes

SWvheerden approved these changes Jan 28, 2022

View reviewed changes

stringhandler merged commit 8d9c7f4 into tari-project:development Jan 28, 2022

sdbondi deleted the core-hash-fix branch January 28, 2022 12:25

sdbondi restored the core-hash-fix branch February 3, 2022 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(validators): check input metadata matches spending output #3761

fix(validators): check input metadata matches spending output #3761

sdbondi commented Jan 27, 2022 •

edited

Loading

SWvheerden left a comment

SWvheerden Jan 28, 2022

sdbondi Jan 28, 2022

SWvheerden Jan 28, 2022

SWvheerden Jan 28, 2022

SWvheerden Jan 28, 2022

stringhandler commented Jan 28, 2022

	debug!(target: LOG_TARGET, "New template block: {}", block_template);
	trace!(target: LOG_TARGET, "New template block: {}", block_template);

	debug!(target: LOG_TARGET, "Miner submitted block: {}", block);
	trace!(target: LOG_TARGET, "Miner submitted block: {}", block);

	debug!(target: LOG_TARGET, "Prepared block: {}", block_template);
	trace!(target: LOG_TARGET, "Prepared block: {}", block_template);

	debug!(target: LOG_TARGET, "Incoming block: {}", block);
	trace!(target: LOG_TARGET, "Incoming block: {}", block);

fix(validators): check input metadata matches spending output #3761

fix(validators): check input metadata matches spending output #3761

Conversation

sdbondi commented Jan 27, 2022 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

SWvheerden left a comment

Choose a reason for hiding this comment

SWvheerden Jan 28, 2022

Choose a reason for hiding this comment

sdbondi Jan 28, 2022

Choose a reason for hiding this comment

SWvheerden Jan 28, 2022

Choose a reason for hiding this comment

SWvheerden Jan 28, 2022

Choose a reason for hiding this comment

SWvheerden Jan 28, 2022

Choose a reason for hiding this comment

stringhandler commented Jan 28, 2022

sdbondi commented Jan 27, 2022 •

edited

Loading