0xMiden / 0xMiden/protocol

Add API for transaction re-execution

Đang mở
#2,293 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
rust
Ngôn ngữ chính
Rust
Star
132
Fork
167
Merge trung bình
1 ngày 23 giờ
Pull request đã merge (30 ngày)
110

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.