perf(trie): deduplicate already fetched prefetch targets

[Rjected]

This prevents fetching proofs that have already been fetched by state updates or other proof fetches. Previously we would only extend self.fetched_proof_targets, without checking it or modifying for prefetch calls.

[shekhirin]

am I reading this wrong, or it will leave in targets only either accounts that do not exist in self.fetched_proof_targets, or accounts with storages that are equal to self.fetched_proof_targets?

Think it should be

Suggested change

	.is_none_or(|fetched_storage| fetched_storage == target_storage)
	.is_none_or(|fetched_storage| fetched_storage != target_storage)

[shekhirin]

or maybe rather it should be checking for target_storage being a subset of fetched_storage, and then removing it if so?

[Rjected]

Yeah, I think checking if it's a subset and removing makes more sense here

[shekhirin]

unsure about this

if the account exists in both targets and self.fetched_proof_targets , but has no associated storage slots in either, we should remove this account from targets, because it was already fetched

[shekhirin]

this may result in an empty target_storage, and then we will need to remove the entry from the targets map completely to prevent fetching the account proof

[shekhirin]

Left some comments about logic, I think basically it should be very similar to

reth/crates/engine/tree/src/tree/root.rs

Lines 855 to 886 in a63f92e

	/// Returns accounts only with those storages that were not already fetched, and
	/// if there are no such storages and the account itself was already fetched, the
	/// account shouldn't be included.
	fn get_proof_targets(
	state_update: &HashedPostState,
	fetched_proof_targets: &MultiProofTargets,
	) -> MultiProofTargets {
	let mut targets = MultiProofTargets::default();
	// first collect all new accounts (not previously fetched)
	for &hashed_address in state_update.accounts.keys() {
	if !fetched_proof_targets.contains_key(&hashed_address) {
	targets.insert(hashed_address, HashSet::default());
	}
	}
	// then process storage slots for all accounts in the state update
	for (hashed_address, storage) in &state_update.storages {
	let fetched = fetched_proof_targets.get(hashed_address);
	let mut changed_slots = storage
	.storage
	.keys()
	.filter(|slot| !fetched.is_some_and(|f| f.contains(*slot)))
	.peekable();
	if changed_slots.peek().is_some() {
	targets.entry(*hashed_address).or_default().extend(changed_slots);
	}
	}
	targets
	}

[shekhirin]

LGTM, that looks correct, but would be nice to have some sanity tests

[Rjected]

added some sanity tests