Move away from the #[maybe_async] usage, avoid TransactionProver#1122
Move away from the #[maybe_async] usage, avoid TransactionProver#1122
#[maybe_async] usage, avoid TransactionProver#1122Conversation
drahnr
changed the title
#[maybe_async] usage, avoid TransactionProvider#[maybe_async] usage, avoid TransactionProver
drahnr
force-pushed
the
bernhard-async-experimental
branch
from
b5a3111 to
dd42762
Compare
This reverts commit 46b8b82.
drahnr
force-pushed
the
bernhard-async-experimental
branch
from
0539f67 to
a5c3b96
Compare
crates/proto/src/domain/block.rs
Outdated
| 0_u32, | ||
| ) | ||
| .unwrap() | ||
| FeeParameters::new(AccountId::new_unchecked(<[Felt; 2]>::default()), 0_u32).unwrap() |
There was a problem hiding this comment.
Pushed a commit that changed this, turns out AccountId::dummy was behind the testing flag on miden-objects
There was a problem hiding this comment.
Had to push another one because there were other occurrences
|
I believe that 0xMiden/miden-client#1104 should work from this branch. @igamigo - could you confirm? (you probably will need to update the dependencies to point to this branch). So, as the next step, we should get this PR ready to merge into |
| [fee_parameters] | ||
| symbol = "MIDEN" | ||
| verification_base_fee = 0 | ||
|
|
||
| [[fungible_faucet]] | ||
| decimals = 3 | ||
| max_supply = 100_000_000 |
There was a problem hiding this comment.
Let's create an issue for refactoring this to be more inline with something like what is described in #1143 (comment).
| async move { | ||
| async move { |
There was a problem hiding this comment.
Question: why do we need nested async moves here?
There was a problem hiding this comment.
If you use the variant with TryFutureExt::inspect_err rather than the Result::inspect_err, the instrument proc_macro
error[E0277]: the trait bound `Result<_, _>: FutureMaybeSend<Result<(), NtxError>>` is not satisfied
--> crates/ntx-builder/src/transaction.rs:70:10
|
66 | #[instrument(target = COMPONENT, name = "ntx.execute_transaction", skip_all, err)]
| ---------------------------------------------------------------------------------- return type was inferred to be `Result<_, _>` here
...
70 | ) -> impl FutureMaybeSend<NtxResult<()>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `futures::Future` is not implemented for `Result<_, _>`
|
is unhappy, since it generates a Result type from it, rather than a future, which causes compile issues.
The trailing async move {..} keeps the heuristic happy and compiles.
| let notes = Box::pin(self.filter_notes(&data_store, notes)).await?; | ||
| let executed = Box::pin(self.execute(&data_store, notes)).await?; | ||
| let proven = Box::pin(self.prove(executed)).await?; |
There was a problem hiding this comment.
Question: is there an alternative to these Box::pin() or do we have to keep using them from now on?
There was a problem hiding this comment.
We do get clippy warnings due to future sizes, ``Box::pin` shifts pieces of it to the heap, which makes the wrapping future smaller and quiets the clippy warnings. Out future sizes exceeded 100 kBytes, which is already pretty heavy, mostly from having to carry plenty of stack residing state across await points.
| #[error("missing fee faucet for native asset {0}")] | ||
| MissingFeeFaucet(String), | ||
| #[error("fee error")] | ||
| FeeError(#[from] FeeError), | ||
| #[error("faucet account of {0} is not a fungible faucet")] | ||
| NativeAssetFaucetIsNotPublic(String), | ||
| #[error("faucet account of {0} is not public")] | ||
| NativeAssetFaucitIsNotAFungibleFaucet(String), |
There was a problem hiding this comment.
I think most of these variants fall away if we make the native faucet a special section in the genesis toml file.
Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com>
4 participants
Depends on
delete traitTransactionProver, avoid#[maybe_async]protocol#1666[async exp] rename usage ofAsyncHostFuturetoFutureMaybeSendprotocol#1693Contains changes from:
FeeParametersintoGenesisConfig#1143