Verify important block invariants (specifically in finality and QC extensions) for Legacy and Transition blocks

[arhag]

Background and suggest protocol compatible simplifications to verify_qc_claim

verify_qc_claim was designed to work whether no matter whether the block it was called on was a Legacy block, Transition block, or Proper Savanna block.

But it turns out the current implementation only calls verify_qc_claim on Proper Savanna blocks (in fact it, from a type perspective, it cannot even call it on a Legacy block).

spring/libraries/chain/controller.cpp

Lines 4006 to 4011 in 21cec1f

	if constexpr (savanna_mode) {
	// Verify claim made by finality_extension in block header extension and
	// quorum_certificate_extension in block extension are valid.
	// This is the only place the evaluation is done.
	verify_qc_claim(id, b, prev);
	}

But consider this part of verify_qc_claim:

spring/libraries/chain/controller.cpp

Lines 3897 to 3916 in 21cec1f

	if( !header_ext ) {
	// If there is no header extension, ensure the block does not have a QC and also the previous
	// block doesn't have a header extension either. Then return early.
	// ------------------------------------------------------------------------------------------
	EOS_ASSERT( !qc_extension_present,
	invalid_qc_claim,
	"Block #${b} includes a QC block extension, but doesn't have a finality header extension",
	("b", block_num) );
	EOS_ASSERT( !prev_finality_ext,
	invalid_qc_claim,
	"Block #${b} doesn't have a finality header extension even though its predecessor does.",
	("b", block_num) );
	dlog("received block: #${bn} ${t} ${prod} ${id}, no qc claim, previous: ${p}",
	("bn", block_num)("t", b->timestamp)("prod", b->producer)("id", id)("p", b->previous));
	return;
	}
	assert(header_ext);

If verify_qc_claim is only called for Proper Savanna blocks, then that code can be simplified to just an EOS_ASSERT that checks that header_ext is present.

Furthermore, consider this part of verify_qc_claim:

spring/libraries/chain/controller.cpp

Lines 3928 to 3934 in 21cec1f

	if (!prev_finality_ext) {
	EOS_ASSERT( !qc_extension_present && new_qc_claim.block_num == block_num && new_qc_claim.is_strong_qc == false,
	invalid_qc_claim,
	"Block #${b}, which is the finality transition block, doesn't have the expected extensions",
	("b", block_num) );
	return;
	}

If verify_qc_claim is only called for Proper Savanna blocks, then that code can also be simplfied to just an EOS_ASSERT that checks that prev_finality_ext is present.

The code is still safe for blocks other Proper Savanna blocks, especially when considering there is also the later check that happens in block_header_state::next:

spring/libraries/chain/block_header_state.cpp

Lines 442 to 443 in 21cec1f

	EOS_ASSERT(f_entry != exts.end(), invalid_block_header_extension,
	"Instant Finality Extension is expected to be present in all block headers after switch to IF");

But the above simplifications are valuable to make the code clearer.

Undesired blockchain validation rules in existing Spring implementation

None of the above so far is an actual bug. However, the fact that verify_qc_claim is not called for Legacy or Transition blocks means that the current implementation of Spring will accept Legacy blocks (i.e. blocks without a finality block header extension) and Transition blocks (i.e. block with a finality block header extension but that has a schedule_version that is not equal to the proper_svnn_schedule_version constant) even if they have a QC block extension present. In fact, there are other block invariants that can be violated for Legacy and Transition blocks that the current Spring implementation does not check for but really should.

The scope of this issue is to ensure that all relevant block invariants for blocks of the different block types (Legacy, Transition, and Proper Savanna) are validated in create_block_state_i prior to constructing the block state.

The relevant block invariants are described below.

Block invariants to validate for the different types of blocks (Legacy, Transition, Proper Savanna)

A Legacy block is defined by not having a finality block header extension.

There are a simple set of checks that are needed for Legacy blocks only:

• The parent block of a Legacy block must also be a Legacy block (i.e. the parent block must not have a finality block header extension present).
• A Legacy block must not have a finality block header extension present. (Trivially satisfied by definition of Legacy block.)
• A Legacy block must not have a schedule_version set to the proper_svnn_schedule_version constant.
• A Legacy block must not have a QC block extension present.

A Transition block is defined by a block that has a finality block header extension present but does not have a schedule_version set to the proper_svnn_schedule_version constant (i.e. is_proper_svnn_block() returns false).

Then there are a set of checks that need to be done for Transition blocks:

• The parent block of a Transition block must either be a Legacy block or a Transition block. Either way the parent block cannot be a Proper Savanna block, which means is_proper_svnn_block() called on the parent block must return false.
• A Transition block must have a finality block header extension present. (Trivially satisfied by definition of Transition block.)
• A Transition block must not have a schedule_version set to the proper_svnn_schedule_version constant. (Trivially satisfied by definition of Transition block.)
• A Transition block must not have a QC block extension present.
• A Transition block must not have a new_proposer_policy_diff present.
• If the parent block of a Transition block is a Legacy block (i.e. a block that does not have a finality block header extension present), then the Transition block is also the Savanna Genesis block. And in that case, the finality block header extension must have a valid genesis finalizer policy present as a diff in the new_finalizer_policy_diff member, and the QC claim in the finality block header extension must be a weak QC claim on the Savanna Genesis block itself.
• If the parent block of a Transition block is another Transition block (i.e. a block that does have a finality block header extension present), then the QC claim of this block must be exactly the same as the QC claim of its parent block and there should be no new_finalizer_policy_diff present.

A Proper Savanna block is defined as a block that has a schedule_version set to the proper_svnn_schedule_version constant (i.e. is_proper_svnn_block() returns true). This also necessarily implies that it has a finality block header extension present (assuming it was a valid block).

Then there are a set of basic checks that need to be done for Proper Savanna blocks:

• The parent block of a Proper Savanna block must either be Transition block or a Proper Savanna block. Either way the parent block cannot be a Legacy block, which means the parent block must have a finality header extension present.
• A Proper Savanna block must have a finality block header extension present. (Trivially satisfied by definition of Proper Savanna block.)
• A Proper Savanna block must have a schedule_version set to the proper_svnn_schedule_version constant. (Trivially satisfied by definition of Proper Savanna block.)
• The QC claim of the parent block must not be any better than the QC claim of the current block. This means that the block number of the QC claim of the parent block is either less than that of the current block, or if they have the same block number, then it is not allowed for the parent block to claim a strong QC while the current block claims a weak QC.
• If the QC claim of the current block is exactly the same as the QC claim of the parent block, then the current block must not have a QC block extension present.
• If the QC claim of the current block is not exactly the same as the QC claim of the parent block, then the current block must have exactly one QC block extension present. The only validation that should be done on that QC block extension at this point is that when calling to_qc_claim() on the QC, the same claim (block number and weak/strong status) is returned as what is in the finality block header extension.

Finally, there is complete QC validation that needs to only be done for Proper Savanna blocks that have a QC block extension present:

• Validate the QC attached in the QC block extension. This requires knowing the finality digest, active finalizer policy, and pending finalizer policy (if present) of the block that is claimed.

Protocol compatibility implications

Technically, to prevent such undesired Legacy and Transition blocks from being accepted would require changes to the implementation which amount to a blockchain validation protocol change compared to Spring 1.0.0-rc1, 1.0.0-rc2, and 1.0.0-rc3. But the reality is that the unmodified nodeos implementation would not produce blocks that violate any of these invariants. And furthermore, to violate almost all of these invariants in a produced block, it would take a producer (a somewhat trusted entity in most realizations of Antelope chains) to run a modified version of the nodeos implementation. The one exception is that a man-in-the-middle could add a superfluous (and incorrect) QC block extension to Legacy and Transition blocks that would be accepted (and propagated) by the current nodeos implementation and potentially be written out to the irreversible block log. But even the block validation protocol was to be made stricter in the future, it would be possible to prune out these undesired superfluous QCs from the irreversible blocks log.

So it is extremely unlikely for the protocol change (which is still allowed in release candidate stage) to cause problems with any existing (test) chains out there that have already deployed prior release candidates of 1.0.0 or even already transition to Savanna.

[heifner]

But it turns out the current implementation only calls verify_qc_claim on non-legacy blocks (so IF Transition Blocks and Proper IF Blocks):

It is not called on IF Transition Blocks as IF Transition Blocks are only transmitted as legacy blocks and then transitioned on each node from legacy to savanna. The verify_qc_claim is not called on the IF Transition Blocks as they are created in the node.

The blocks are Transition blocks. I was thinking of block_state. The verify qc claim is not called on block_state of transition blocks. So ignore this comment.