a fork removing the start of savanna transition will cause a state history failure with finality-data-history enabled

[spoonincode]

Notice how ship plugin writes to the finality data log,

spring/plugins/state_history_plugin/state_history_plugin.cpp

Lines 189 to 200 in 391be54

	void store_finality_data(const block_id_type& id, const block_id_type& previous_id) {
	if(!finality_data_log)
	return;
	std::optional<finality_data_t> finality_data = chain_plug->chain().head_finality_data();
	if(!finality_data.has_value())
	return;
	finality_data_log->pack_and_write_entry(id, previous_id, [finality_data](bio::filtering_ostreambuf& buf) {
	fc::datastream<boost::iostreams::filtering_ostreambuf&> ds{buf};
	fc::raw::pack(ds, *finality_data);
	});

If a finality_data_t is returned, it is written to the finality data log. Otherwise, nothing is done with the log.

Remember though that ship log entries must be continuous: there can be no gaps in them.

Consider that savanna transition begins on block 200, and continues for another block 201. This will write two entries in to ship's finality data log: block 200 and 201. Now consider that a new fork is switched to back on block 198, and that this fork continues without savanna activation; say it continues on to block 220.

At this point, ship will continue to indicate a finality data log of blocks 200 through 201 and even return finality data for those blocks even though that never happened from the standpoint of the current fork being followed.

But then consider on block 221 there is another attempt to transition to savanna. This will cause ship to attempt to write to block 221 of the finality log, but this will be flagged as a discontinuity in ship due to the gap between 201 and 221. Ultimately this will cause the node to shut down.

[linh2931]

Should we stop sending finality_data of transitional blocks, and only send them after transition to Savanna is finalized? Need some mechanism to coordinate SHiP and the controller.

[spoonincode]

My initial thought at two easy ways to potentially resolve the risk:

1. always write out the entire std::optional<finality_data_t>, or
2. add a new clear() to the log/log_catalog, which would look something like,

if(!finality_data.has_value()) {
   finality_data_log->clear();
   return; 
}

(clear() could be quite fast and straightforward if log is already empty() -- just do nothing)

The second option is more "forward looking" I think, since finality_data_t will nominally always be around going forward.