Remove `TransactionInputs::asset_witnesses `

[igamigo]

Discussed in 0xMiden/node#1490

TransactionInputs::asset_witnesses only contains prefetched asset witnesses (note assets + fee asset).
On re-execution, AFAICT, dynamic calls to DataStore::get_vault_asset_witnesses() are skipped because the Merkle paths are already in advice data, but prefetched witnesses are still fetched every time, so DataStore implementors .
This introduces an inconsistent, under-documented behavior and forces validator implementations to special-case prefetched assets.

Keeping this in mind, we could remove TransactionInputs::asset_witnesses in favor of adding the witnesses directly in TransactionInputs::with_asset_witnesses as suggested here 0xMiden/node#1490 (comment) and consider skipping prefetch on re-execution by checking advice inputs before fetching.

[Farukest]

Hey @bobbinth @igamigo
I'd like to work on this

[bobbinth]

Hi @Farukest! Thank you! I've assigned this to you.

[PhilippGackstatter]

I agree the get_asset_vault_witnesses call should be possible to be skipped if all witnesses are already there.

I think in prepare_tx_inputs, we can filter the asset_vault_keys by tx_args.advice_inputs().store.has_path(...), and only call get_asset_vault_witnesses if !asset_vault_keys.is_empty().

[Farukest]

Thanks @PhilippGackstatter, for the suggestion. Just to clarify; should this optimization be part of this PR or a separate follow up?

[PhilippGackstatter]

I think we can do both changes (removing witnesses, and skipping the request if witnesses are already present) in one PR. They should be relatively small.