BlockValidationState is discarded when processing header fails

[stringintech]

py-bitcoinkernel/src/pbk/chain.py

Lines 510 to 528 in b4126a0

	def process_block_header(self, header: "BlockHeader") -> "BlockValidationState":
	"""
	Processes and validates the provided block header.
	Args:
	header: block header to be processed
	Returns:
	The result of the block header validation
	Raises:
	ProcessBlockHeaderException: If processing the block header failed. Duplicate block headers do not throw.
	"""
	state = BlockValidationState()
	result = k.btck_chainstate_manager_process_block_header(self, header, state)
	if result != 0:
	raise ProcessBlockHeaderException(result)
	return state

It seems to me that state provides additional info for failure cases (when result != 0) not the other way?

[stickies-v]

Thanks for flagging this, you're right! I misunderstood the current api, I thought non-zero return codes were returned when processing failed (my interpretation of the docs was for this to be an unhandled exception), but the implementation shows it is also returned for invalid states:

int btck_chainstate_manager_process_block_header(
    btck_ChainstateManager* chainstate_manager,
    const btck_BlockHeader* header,
    btck_BlockValidationState* state)
{
    try {
        auto& chainman = btck_ChainstateManager::get(chainstate_manager).m_chainman;
        auto result = chainman->ProcessNewBlockHeaders({&btck_BlockHeader::get(header), 1}, /*min_pow_checked=*/true, btck_BlockValidationState::get(state), /*ppindex=*/nullptr);

        return result ? 0 : -1;
    } catch (const std::exception& e) {
        LogError("Failed to process block header: %s", e.what());
        return -1;
    }
}

I'm not sure the current C API implementation is the best it can be, so going to leave this open for a while to see whether the C API or pbk needs changing.