state: propagate errors when removing transparent index entries

[jonwalch]

Summary

While investigating #681, I noticed that the transparent index range-delete error is ignored during index removal:

zakura/crates/zakura-state/src/service/finalized_state/disk_format/upgrade/drop_tx_locs_by_spends.rs

Lines 26 to 33 in f892b90

	let _ = zakura_db
	.tx_loc_by_spent_output_loc_cf()
	.new_batch_for_writing()
	.zs_delete_range(
	&crate::OutputLocation::from_output_index(crate::TransactionLocation::MIN, 0),
	&crate::OutputLocation::from_output_index(crate::TransactionLocation::MAX, u32::MAX),
	)
	.write_batch();

write_batch() returns a Result<(), rocksdb::Error>, but let _ = discards the result and any RocksDB error:

zakura/crates/zakura-state/src/service/finalized_state/column_family.rs

Lines 319 to 323 in f892b90

	/// Writes this batch to this column family in the database,
	/// taking ownership and consuming it.
	pub fn write_batch(self) -> Result<(), rocksdb::Error> {
	self.inner.db.write(self.batch)
	}

Impact

If the range deletion fails transiently and later writes succeed, drop_tx_locs_by_spends::run() can continue replacing shielded nullifier locations with empty values and then return success. Its caller subsequently removes the +indexer marker even though transparent index entries remain readable from the database:

zakura/crates/zakura-state/src/service/finalized_state/disk_format/upgrade.rs

Lines 595 to 602 in f892b90

	info!("started removing indexes for spending tx ids");
	drop_tx_locs_by_spends::run(initial_finalized_tip_height, db, cancel_receiver)?;
	info!("finished removing indexes for spending tx ids");
	// Remove build metadata to on-disk version file after indexes have been dropped.
	version.build = db.format_version_in_code().build;
	db.update_format_version_on_disk(&version)
	.expect("unable to write database format version file to disk");

This can also affect a later index rebuild. track_tx_locs_by_spends treats the first existing spend mapping it encounters for a height as proof that the entire height was indexed:

zakura/crates/zakura-state/src/service/finalized_state/disk_format/upgrade/track_tx_locs_by_spends.rs

Lines 48 to 64 in f892b90

	if !should_index_at_height {
	if let Some(spend) = tx
	.inputs()
	.iter()
	.filter_map(|input| Some(input.outpoint()?.into()))
	.chain(tx.sprout_nullifiers().cloned().map(Spend::from))
	.chain(tx.sapling_nullifiers().cloned().map(Spend::from))
	.chain(tx.orchard_nullifiers().cloned().map(Spend::Orchard))
	.chain(tx.ironwood_nullifiers().cloned().map(Spend::Ironwood))
	.next()
	{
	if read::spending_transaction_hash::<Arc<Chain>>(None, zakura_db, spend)
	.is_some()
	{
	// Skip transactions in blocks with existing indexes
	return Ok(());
	} else {

A leftover transparent mapping can therefore make the rebuild incorrectly assume that the entire block was already indexed, causing it to skip restoring missing shielded index entries.

Persistent storage failures will normally cause subsequent writes to fail as well, so this is a latent correctness and recovery issue rather than evidence of observed database corruption. RocksDB can, however, automatically recover from eligible retryable I/O and WAL errors, so the removal code should not rely on every later write failing after the initial error.

Expected behavior

Index removal must not be reported as complete until every deletion and rewrite has succeeded. A storage error should preserve a state that causes removal to be retried on the next writable startup.

Acceptance criteria

• Propagate the initial transparent range-delete error instead of discarding it.
• Do not remove the +indexer marker, or persist a future Absent state, when any removal write fails.
• Retry interrupted or failed removal on the next writable startup.
• Add regression coverage for a transient failure of the initial range deletion followed by otherwise successful writes.
• Verify that a later index rebuild cannot skip a height containing partially removed index data.

Related: #681