0xMiden / 0xMiden/protocol

Add API for transaction re-execution

未关闭
#2,293 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
rust
主要语言
Rust
星标
132
派生
167
平均合并
1 天 23 小时
30 天内合并 PR
110

描述

Re-execution of a transaction currently requires using `TransactionExecutor::execute_transaction`. This is an unnecessary hurdle because it requires a `DataStore` and a `TransactionAuthenticator` impl. The only thing that should be necessary is a `MastForestStore` impl as required by the VM.

If I understand correctly, re-execution of transactions during _proving_ will go away once the necessary VM APIs are exposed, but re-execution itself is basically a solved problem in `LocalTransactionProver::prove`, which doesn't require a `DataStore` or `TransactionAuthenticator`. We should be able to repurpose the way this is set up and the prover host implementation for the purpose of a re-execution API in `miden-tx` that miden node would be able to use.

I think a separate implementation like this is preferable to re-using `TransactionExecutor` for this, as it would be overloaded with the concerns of execution and re-execution, which would introduce complication at various points:
- Foreign account loading happens only when the kernel emits the `miden::account::before_foreign_load` event. Skipping this just based on whether the foreign account id's advice map key is present isn't enough. Additionally:
- the foreign account code from `TransactionInputs::foreign_account_code` would have to be loaded during tx preparation.
- the account procedure index map would have to be built with `TransactionInputs::foreign_account_code` as well. Currently it isn't because this field is assumed to be empty when executing.
- The `DataStore` interface is not designed for re-execution and many of its methods shouldn't be called during re-execution since all data should already be present. However, building an API on top of `TransactionExecutor` for re-execution requires implementing these, and we'd have to do this in some crude way, e.g. always returning errors à la "should not be called".
- We'll probably have to do more things like [this one](https://github.com/0xMiden/miden-base/pull/2274/#discussion_r2697736658) in the tx executor to make it fit for re-execution.

Separately, it seems like `TransactionInputs` has too many concerns at once that make it harder to understand. That seems like a consequence of it being used for execution and re-execution (mentioned also by @igamigo [here](https://github.com/0xMiden/miden-node/pull/1493#discussion_r2695547037)):
- `TransactionInputs::foreign_account_code` being empty _before_ execution, but containing code _after_ execution.
- Advice inputs being overwritten with the contents from the advice provider _after_ execution.

One initial idea to address these two is to bring back `TransactionWitness` for the purpose of re-execution to make this concern clearer. `ExecutedTransaction` would again be convertible into `TransactionWitness` and miden-node submit tx API would take this as an input next to a `ProvenTransaction` for the purpose of re-execution. We can then add `TransactionWitness::execute(&self) -> Result` to re-execute. As mentioned in the beginning, this would take the `LocalTransactionProver` infrastructure as a basis for re-execution. This is just an initial idea, though.

Context: https://github.com/0xMiden/miden-node/pull/1493#discussion_r2689762636

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。