Vectorize transfers #3356
Vectorize transfers #3356
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3356 +/- ##
==========================================
- Coverage 53.92% 53.78% -0.15%
==========================================
Files 317 317
Lines 107575 107837 +262
==========================================
- Hits 58011 58001 -10
- Misses 49564 49836 +272 ☔ View full report in Codecov by Sentry. |
crates/apps_lib/src/client/tx.rs
Outdated
| break; | ||
| } else { | ||
| sign(namada, &mut tx, &args.tx, signing_data).await?; | ||
| let cmt_hash = tx.first_commitments().unwrap().get_hash(); |
There was a problem hiding this comment.
I guess this relates to your comment above that vectorization is not really supported in the cli atm.
There was a problem hiding this comment.
Actually this has to do with transaction batching
| )) | ||
| } | ||
|
|
||
| /// Build a shielded transfer transaction from the given parameters | ||
| pub fn shielded( |
There was a problem hiding this comment.
how out of data is our c bindings example?
crates/sdk/src/masp.rs
Outdated
| } | ||
| // Load the current shielded context given the spending key we | ||
| // possess | ||
| let mut shielded = context.shielded_mut().await; |
There was a problem hiding this comment.
I think we only need to do this once. So we can move it outside of the loop
There was a problem hiding this comment.
We actually want to load the context only if the source is a spending key, which we can know only from within the loop. But I've added a boolean in 937a972 to avoid loading the context more than once
grarco
force-pushed
the
grarco/vectorize-transfers
branch
from
937a972
to
1e98403
Compare
* grarco/vectorize-transfers: Changelog #3356 Fmt Avoids reloading shielded context Misc improvements to signing for vectorized transfers Check no vectorized transfers in cli Fixes signature generation for vectorized transfers Fixes benchmarks Vectorizes masp transfers Vectorizes transparent transfers
* grarco/vectorize-transfers: Changelog #3356 Fmt Avoids reloading shielded context Misc improvements to signing for vectorized transfers Check no vectorized transfers in cli Fixes signature generation for vectorized transfers Fixes benchmarks Vectorizes masp transfers Vectorizes transparent transfers
|
@grarco Not sure if this is strictly necessary, but do any of the wasms in this PR allow a transfer with transparent inputs, transparent outputs, and shielded (inputs or outputs) all there simultaneously? |
Not really, it's more focused on vectorizing the 4 different types of transfers: so shielded transfers are already vectorized since they can contain multiple spend/output notes. Transparent transfers have been updated to support multiple sources/targets/tokens. Shielding has been updated to support multiple transparent sources and unshielding has been updated to support multiple transparent outputs, but there's no way yet to mix them in a single transaction. |
Describe your changes
Closes #2596.
Required modification to enable #2597.
Modifies transparent and masp transactions to allow for multi-party transfers. Transparent transfers allow for multiple sources, targets token and amounts.
Shielding and unshielding masp transfers are instead vectorized in a slightly different way: shieldings are vectorized in a fan-in fashion: multiple sources, amounts and tokens but the target is a single
PaymentAddress. Unshieldings are fan-out: multiple targets, tokens and amount but the source is a singleSpendingKey.Ibc shielded actions remain unchanged as I believe we can't vectorize them because of the way packets are designed.
This PR does NOT update the client to support vectorized transfers as at the moment it's unclear to me how to achieve that maintaining a decent UX: for now we only support these transactions via the SDK.
Indicate on which release or other PRs this topic is based on
v0.39.0Checklist before merging to
draft