Tx batching #3103

grarco · 2024-04-21T17:42:30Z

Describe your changes

Closes #1356.

Implements transaction batching by extending the Tx type to carry a set of commitments to data, code and memo. Every tx now is a batch (the default case is a batch of a single inner tx) and can be set to be an atomic batch if needed. In summary the changes are:

Iterate on all the commitments and execute every inner tx in sequence. Precommit or drop the changes of an inner tx (based on vps/errors) before launching the next one to ensure that every tx sees the correct state
If the batch is atomic and a single inner tx fails, we discard all the changes, even those coming from inner txs that were successful
A single gas meter is used for the entire batch
Replay protection and inner tx signatures are managed at the batch Header level, as in we sign the entire batch header (which contains the commitments of all the txs). This way the logic is simplified and the transaction can carry less signatures
WriteLog and TxResult have been updated to support this new logic
The Event emitted by the transaction has been updated to carry the results of all the inner txs of the batch
Added a new method in the sdk to construct a batch

This is a first attempt to achieve tx batching but the feature should not be considered stabilized yet. More specifically I believe more work will be needed to:

Refactor some parts of the code base that might have become too intricated (especially the evalute_tx_result function in finalize_block)
Improve the logic to build transactions in the sdk and add some support in the client (currently lacking)
Add at least one integration test to test the entire loop
More unit tests (ideally one testing gas metering in a batch and more unit tests for the updated write log)
Review/refactor the events
Update Hermes (all the tests failing)
Investigate supporting batching from cli

Indicate on which release or other PRs this topic is based on

v0.35.1

Checklist before merging to `draft`

I have added a changelog
Git history is in acceptable state

…xecution

…ock`

codecov · 2024-04-28T16:36:10Z

Codecov Report

Attention: Patch coverage is 49.29633% with 1297 lines in your changes are missing coverage. Please review.

Project coverage is 60.40%. Comparing base (4ed6229) to head (97a2d8d).

Files	Patch %	Lines
crates/sdk/src/signing.rs	0.00%	717 Missing ⚠️
crates/sdk/src/tx.rs	0.00%	88 Missing ⚠️
crates/namada/src/ledger/protocol/mod.rs	57.22%	77 Missing ⚠️
crates/sdk/src/masp.rs	0.00%	71 Missing ⚠️
crates/tx/src/types.rs	57.57%	56 Missing ⚠️
crates/apps/src/lib/bench_utils.rs	0.00%	52 Missing ⚠️
...s/apps/src/lib/node/ledger/shell/finalize_block.rs	89.64%	47 Missing ⚠️
crates/sdk/src/rpc.rs	0.00%	46 Missing ⚠️
crates/events/src/extend.rs	0.00%	13 Missing ⚠️
crates/apps/src/lib/node/ledger/shell/mod.rs	81.96%	11 Missing ⚠️
... and 25 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3103      +/-   ##
==========================================
+ Coverage   60.24%   60.40%   +0.16%     
==========================================
  Files         303      303              
  Lines       93191    94176     +985     
==========================================
+ Hits        56145    56890     +745     
- Misses      37046    37286     +240

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

crates/apps/src/lib/node/ledger/shell/finalize_block.rs

murisi

The changes to the Tx format look good to me. Given that the only change in the format is that the data, code, and memo hashes are vectorized, hardware wallet support for singleton batch transactions (where batch.len() == 1) should be quickly achievable. But it will probably take some time to implement support non-singleton batch transactions because the hardware wallet codebase wasn't structured for holding, printing, and signing multiple transactions in one go.

murisi · 2024-05-15T13:53:02Z

wasm_for_tests/tx_invalid_data/src/lib.rs

+        ref cmt,
+    } = tx_data;
+    let _data =
+        signed


Probably BatchedTx would benefit from a fn data(&self) -> Option<Vec<u8>> { ... } method. This signed.data(cmt) pattern (where cmt and signed come from the same BatchedTx) seems to be repeated somewhat.

I believe this will be addressed (in some way) by #3200

murisi · 2024-05-15T14:42:51Z

crates/tx/src/types.rs

+
+    /// Add a new inner tx to the transaction. Returns `false` if the
+    /// commitments already existed in the collection. This function expects a
+    /// transaction carrying a single inner tx as input


Is there a reason this function is limited to a single inner tx?

At the moment the support to build batched transactions is very limited. We'll probably want to rework the way we construct txs so for now I've implemented something minimal. The caller can invoke this function multiple times if needed

murisi · 2024-05-15T14:46:04Z

crates/tx/src/types.rs

+    /// Add a new inner tx to the transaction. Returns `false` if the
+    /// commitments already existed in the collection. This function expects a
+    /// transaction carrying a single inner tx as input
+    pub fn add_inner_tx(&mut self, other: Tx, cmt: TxCommitments) -> bool {


If this function is limited to a single inner tx, then I guess the pair (other, cmt) is conceptually a BatchedTx...

Ah you are right! Still, I think it would just be better to rework this function completely in the near future

* grarco/tx-batch: Restores `batch_size` arg for `fetch` Refactors `dispatch_tx` to return a result Refactors masp events to avoid dynamic attributes Short-circuit atomic batches at the first failure Implements a `get_tx_data` method on `Ctx` Refactors validity booleans into a `ValidityFlags` struct Refactors buckets in write log with iterator chains Misc refactors, fixes typo Improves unit tests for batches Refactors match on inner tx result Preserve txs order when constructing batch in the sdk Drops duplicated events for errored batch More unit tests for tx batching Appends inner tx result even in case of out of gas Better handling of batch errors Restores shielded sync struct in the sdk Reverts wrong shielded sync cli changes Removes broken batch section optimization Extracts ok tx result evaluation to a separate function Changes error code for batches. Fixes missing events Refactors `evaluate_tx_result`. Updates stats Renames `TxInfo` and fixes write log commit in `finalize_block` Refactors stats accounting Reverts `TryFrom` impls for ethereum data to `Tx` Changelog #1356 Adds unit tests for tx batches Updates tx types comparisons and avoid duplicated signatures for batches Updates error message in ibc test Fixes bug in wrapper keys Fixes fee payemnt logic and unit tests Recomputes signatures for localnet genesis Clippy fix Removes unused `Ciphertext` section Updates `add_inner_tx` to avoid duplicated sections Adds new `mk_tx_batch` for unit testing Fixes tx interface to attach a new inner tx Updates comments Adds an SDK function to construct tx batches Renames `Commitments` and update docs Refactors the batch write log Refactors tx result handling in `finalize_block` Removes custom borsh impls for `BatchedTx` and `BatchedTxRef` More refactoring Misc refactoring Fixes benchmarks Fixes integration tests Fixes wasm tests Refactors test envs Fixes serialization of tx result and unit tests Rebuilds wasm for tests Recomputes genesis signatures. Fixes missing commitments in tests Updates `VpEval` Fixes wasm for test. Clippy + fmt Updates wasm interface and codes Renames batched txs structs Misc updates to write log. Updates tx result handling in `finalize_block` Updates `WriteLog` to support tx batches Improves handling of txs' results in `dispatch_tx` and `finalize_block` Introduces `OwnedBatchedTx` for benchmarks. Fixes benches and tests Generic `TxResult` Updates contexts for batched transactions. Misc updates to protocol Updates logs and tx result to support batching Adds `BatchedTx` struct and updated the native vps and transactions execution Updates core tx methods and masp client functions Adds multiple tx commitments in `Header`

* origin/grarco/tx-batch: Restores `batch_size` arg for `fetch` Refactors `dispatch_tx` to return a result Refactors masp events to avoid dynamic attributes Short-circuit atomic batches at the first failure Implements a `get_tx_data` method on `Ctx` Refactors validity booleans into a `ValidityFlags` struct Refactors buckets in write log with iterator chains Misc refactors, fixes typo Improves unit tests for batches Refactors match on inner tx result Preserve txs order when constructing batch in the sdk Drops duplicated events for errored batch More unit tests for tx batching Appends inner tx result even in case of out of gas Better handling of batch errors Restores shielded sync struct in the sdk Reverts wrong shielded sync cli changes Removes broken batch section optimization Extracts ok tx result evaluation to a separate function Changes error code for batches. Fixes missing events Refactors `evaluate_tx_result`. Updates stats Renames `TxInfo` and fixes write log commit in `finalize_block` Refactors stats accounting Reverts `TryFrom` impls for ethereum data to `Tx` Changelog #1356 Adds unit tests for tx batches Updates tx types comparisons and avoid duplicated signatures for batches Updates error message in ibc test Fixes bug in wrapper keys Fixes fee payemnt logic and unit tests Recomputes signatures for localnet genesis Clippy fix Removes unused `Ciphertext` section Updates `add_inner_tx` to avoid duplicated sections Adds new `mk_tx_batch` for unit testing Fixes tx interface to attach a new inner tx Updates comments Adds an SDK function to construct tx batches Renames `Commitments` and update docs Refactors the batch write log Refactors tx result handling in `finalize_block` Removes custom borsh impls for `BatchedTx` and `BatchedTxRef` More refactoring Misc refactoring Fixes benchmarks Fixes integration tests Fixes wasm tests Refactors test envs Fixes serialization of tx result and unit tests Rebuilds wasm for tests Recomputes genesis signatures. Fixes missing commitments in tests Updates `VpEval` Fixes wasm for test. Clippy + fmt Updates wasm interface and codes Renames batched txs structs Misc updates to write log. Updates tx result handling in `finalize_block` Updates `WriteLog` to support tx batches Improves handling of txs' results in `dispatch_tx` and `finalize_block` Introduces `OwnedBatchedTx` for benchmarks. Fixes benches and tests Generic `TxResult` Updates contexts for batched transactions. Misc updates to protocol Updates logs and tx result to support batching Adds `BatchedTx` struct and updated the native vps and transactions execution Updates core tx methods and masp client functions Adds multiple tx commitments in `Header`

* tomas/split-up-apps: changelog: add #3259 test/apps_lib: fix the top-level dir check fix paths for split up namada_apps_lib git: ignore the new generated version.rs path symlink proto from `apps_lib` `mv crates/apps/build.rs crates/apps_lib/` `mv crates/apps_lib/src/lib/mod.rs crates/apps_lib/src/lib.rs` `mv crates/apps/src/lib crates/apps_lib/src` apps_lib: add a new crate for apps lib crate fixup! Merge branch 'grarco/masp-fees' (#3217) fixup! Merge branch 'grarco/tx-batch' (#3103) fixup! Merge branch 'grarco/tx-batch' (#3103) fixup! Merge branch 'bat/fix/issue-1796' (#3226) fixup! Merge branch 'tiago/max-proposal-bytes-validation' (#3220) fixup! Merge branch 'tomas/more-checked-arith' (#3214) empty commit changelog: add #3216 deliberatly empty Changelog #2817 added clone to transaction structs

grarco added 10 commits April 14, 2024 20:42

Adds multiple tx commitments in Header

ac39259

Updates core tx methods and masp client functions

086a8a8

Adds BatchedTx struct and updated the native vps and transactions e…

9bfed1c

…xecution

Updates logs and tx result to support batching

50518ea

Updates contexts for batched transactions. Misc updates to protocol

167204f

Generic TxResult

1da5b0a

Introduces OwnedBatchedTx for benchmarks. Fixes benches and tests

5853fb1

Improves handling of txs' results in dispatch_tx and finalize_block

6374f1a

Updates WriteLog to support tx batches

3a846bd

Misc updates to write log. Updates tx result handling in `finalize_bl…

8fdea42

…ock`

grarco force-pushed the grarco/tx-batch branch from afa7511 to 8fdea42 Compare April 24, 2024 10:53

grarco added 7 commits April 26, 2024 12:46

Renames batched txs structs

7267a43

Updates wasm interface and codes

da34ab9

Fixes wasm for test. Clippy + fmt

af4513d

Updates VpEval

eace62f

Recomputes genesis signatures. Fixes missing commitments in tests

c4d19fa

Rebuilds wasm for tests

b4176e9

Fixes serialization of tx result and unit tests

55dae06

grarco added 11 commits April 28, 2024 18:44

Refactors test envs

2fd7f4c

Fixes wasm tests

b342781

Fixes integration tests

aaf26c4

Fixes benchmarks

07ec96e

Misc refactoring

baac8ec

More refactoring

593be95

Removes custom borsh impls for BatchedTx and BatchedTxRef

a50b4d8

Refactors tx result handling in finalize_block

25bb28c

Refactors the batch write log

dc364c6

Renames Commitments and update docs

db3e688

Adds an SDK function to construct tx batches

4d0d34e

grarco commented May 7, 2024

View reviewed changes

crates/apps/src/lib/node/ledger/shell/finalize_block.rs Outdated Show resolved Hide resolved

grarco added 2 commits May 8, 2024 15:28

Refactors masp events to avoid dynamic attributes

01510ea

Refactors dispatch_tx to return a result

db8fa36

This was referenced May 8, 2024

Review ResultCode::WasmRuntimeError for batch errors #3199

Open

Refactor inner Txs interface #3200

Open

tzemanovic mentioned this pull request May 9, 2024

Draft - released as 0.36.0 #3213

Closed

grarco mentioned this pull request May 9, 2024

Remove wrapper fee unshielding #3217

Merged

2 tasks

Merge branch 'v0.35.1' into grarco/tx-batch

7f4b6c1

cwgoes mentioned this pull request May 14, 2024

Allow multiple transfers in a single tx #2596

Closed

murisi previously approved these changes May 15, 2024

View reviewed changes

murisi reviewed May 15, 2024

View reviewed changes

Restores batch_size arg for fetch

97a2d8d

grarco dismissed murisi’s stale review via 97a2d8d May 16, 2024 09:49

brentstone added a commit that referenced this pull request May 16, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

6d4b17d

grarco added a commit that referenced this pull request May 17, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

1d45b56

tzemanovic pushed a commit that referenced this pull request May 20, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

30bb889

tzemanovic pushed a commit that referenced this pull request May 20, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

1dfa217

tzemanovic added a commit that referenced this pull request May 20, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

387fab1

tzemanovic added the merged to draft label May 20, 2024

brentstone added a commit that referenced this pull request May 21, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

9137491

brentstone added a commit that referenced this pull request May 21, 2024

fixup! Merge branch 'grarco/tx-batch' (#3103)

570aa4c

tzemanovic merged commit c96d1bc into main May 21, 2024
11 of 19 checks passed

tzemanovic deleted the grarco/tx-batch branch May 21, 2024 13:29

jurevans mentioned this pull request May 24, 2024

Extension/Interface: Update to support Tx batching anoma/namada-interface#779

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tx batching #3103

Tx batching #3103

grarco commented Apr 21, 2024 •

edited

Loading

codecov bot commented Apr 28, 2024 •

edited

Loading

murisi left a comment

murisi May 15, 2024

grarco May 16, 2024 •

edited

Loading

murisi May 15, 2024

grarco May 16, 2024

murisi May 15, 2024

grarco May 16, 2024

Tx batching #3103

Tx batching #3103

Conversation

grarco commented Apr 21, 2024 • edited Loading

Describe your changes

Indicate on which release or other PRs this topic is based on

Checklist before merging to draft

codecov bot commented Apr 28, 2024 • edited Loading

Codecov Report

murisi left a comment

Choose a reason for hiding this comment

murisi May 15, 2024

Choose a reason for hiding this comment

grarco May 16, 2024 • edited Loading

Choose a reason for hiding this comment

murisi May 15, 2024

Choose a reason for hiding this comment

grarco May 16, 2024

Choose a reason for hiding this comment

murisi May 15, 2024

Choose a reason for hiding this comment

grarco May 16, 2024

Choose a reason for hiding this comment

grarco commented Apr 21, 2024 •

edited

Loading

Checklist before merging to `draft`

codecov bot commented Apr 28, 2024 •

edited

Loading

grarco May 16, 2024 •

edited

Loading