Account on-chain tracking #485

hackaugusto · 2024-02-27T17:05:05Z

Built on top of: #481

This PR does:

Create a builder for ProvenTransactions
The above builder adds validation for the account and note details
~~Add a flag so that the transaction prover will track the account details~~ Use the on-chain flag from the account id

Will come on follow-ups:

Note details tracking (Looking into how to determine which notes should be tracked)

hackaugusto · 2024-02-28T16:36:40Z

miden-tx/src/prover/mod.rs

+        let builder = match on_chain_account {
+            true => {
+                let account_details = if tx_witness.account().is_new() {
+                    AccountDetails::Full(tx_witness.account().clone())
+                } else {
+                    AccountDetails::Delta(account_delta)
+                };
+
+                builder.account_details(account_details)
            },
-            tx_outputs.account.hash(),
-            input_notes,
-            tx_outputs.output_notes.into(),
-            tx_script_root,
-            block_hash,
-            proof,
-        ))
+            false => builder,
+        };


I'm not particularly fond of this API. But as the things are currently set, this seems to be the best place to have it.

An alternative would be to decouple the ProvenTransaction from the account/note state. But at the same time I don't think that would be worth the effort.

hackaugusto · 2024-03-04T15:52:05Z

@bobbinth ping. In case you missed this PR :)

bobbinth

Thank you! Looks good. I left some comments inline.

Looking into how to determine which notes should be tracked

One option could be to modify TransactionProver::prove_transaction() method to take a vector of notes as an extra parameter (i.e., it would be Vec<Note>).

Another option is to modify TransactionWitness to add something like expected_output_notes field to it. I think this might be cleaner - but I haven't considered all pros/cons.

miden-tx/src/prover/mod.rs

objects/src/transaction/proven_tx.rs

hackaugusto · 2024-03-05T16:54:45Z

One option could be to modify TransactionProver::prove_transaction() method to take a vector of notes as an extra parameter (i.e., it would be Vec).

Another option is to modify TransactionWitness to add something like expected_output_notes field to it. I think this might be cleaner - but I haven't considered all pros/cons.

In the issue it was describe this should be determined by the MASM code. Any reason to change that?

hackaugusto · 2024-03-05T18:43:43Z

closing/reopening to trigger the ci

hackaugusto · 2024-03-06T17:11:09Z

Note: I changed a few tests from using an ON_CHAIN account type to an OFF_CHAIN account type, because this PR does not implement the delta yet.

applied requested changes

hackaugusto · 2024-03-08T14:47:17Z

@bobbinth ping, the PR is ready for another review :)

bobbinth

Thank you! Looks good! I left a few more comments inline - but pretty much all are minor.

Also, could you create an issue mentioned in #485 (comment)?

objects/src/transaction/proven_tx.rs

objects/src/errors.rs

objects/src/transaction/proven_tx.rs

objects/src/errors.rs

objects/src/transaction/proven_tx.rs

bobbinth · 2024-03-08T22:59:29Z

Oh - and let's also update the changelog.

objects/src/transaction/proven_tx.rs

hackaugusto · 2024-03-11T11:35:08Z

Oh - and let's also update the changelog.

added

bobbinth

Looks good! Thank you! I left a couple of small comments inline. After these, let's merge.

objects/Cargo.toml

objects/src/transaction/proven_tx.rs

comments have ben resolved

hackaugusto requested review from phklive and bobbinth February 27, 2024 17:05

hackaugusto force-pushed the hacka-issue-403-2 branch 2 times, most recently from 7a4a711 to dabbbc8 Compare February 28, 2024 15:21

hackaugusto commented Feb 28, 2024

View reviewed changes

hackaugusto force-pushed the hacka-issue-403-2 branch from dabbbc8 to bdb7b8e Compare February 28, 2024 16:38

This was referenced Mar 4, 2024

Implement support for on-chain notes 0xPolygonMiden/miden-node#141

Closed

Implement support for on-chain accounts 0xPolygonMiden/miden-node#142

Closed

bobbinth previously requested changes Mar 5, 2024

View reviewed changes

hackaugusto force-pushed the hacka-issue-403-2 branch from bdb7b8e to e1c6e32 Compare March 5, 2024 16:51

hackaugusto force-pushed the hacka-issue-403-2 branch from e1c6e32 to 1fbafe9 Compare March 5, 2024 18:09

hackaugusto changed the base branch from main to next March 5, 2024 18:09

hackaugusto closed this Mar 5, 2024

hackaugusto reopened this Mar 5, 2024

hackaugusto force-pushed the hacka-issue-403-2 branch 5 times, most recently from afa034e to e9623b4 Compare March 6, 2024 17:10

hackaugusto requested a review from bobbinth March 6, 2024 17:11

hackaugusto mentioned this pull request Mar 8, 2024

Note types #515

Merged

hackaugusto force-pushed the hacka-issue-403-2 branch from e9623b4 to 41aa4ce Compare March 8, 2024 14:46

hackaugusto requested a review from polydez March 8, 2024 15:05

bobbinth requested changes Mar 8, 2024

View reviewed changes

bobbinth force-pushed the next branch from 5369f6b to 0156afd Compare March 8, 2024 23:06

polydez previously requested changes Mar 9, 2024

View reviewed changes

objects/src/transaction/proven_tx.rs Show resolved Hide resolved

objects/src/transaction/proven_tx.rs Show resolved Hide resolved

hackaugusto mentioned this pull request Mar 11, 2024

Fix note serialization #520

Closed

hackaugusto force-pushed the hacka-issue-403-2 branch from decfbbb to 9010e7b Compare March 11, 2024 11:12

hackaugusto added 3 commits March 11, 2024 12:13

objects: add account/note details to ProvenTransaction #403

f3d075a

objects: Add ProvenTransactionBuilder

7316019

tx-prover: Add account on-chain state tracking

e3545f5

hackaugusto force-pushed the hacka-issue-403-2 branch from 9010e7b to fa535cb Compare March 11, 2024 11:14

review: typo fixes

8c6fefc

hackaugusto force-pushed the hacka-issue-403-2 branch from fa535cb to 8c6fefc Compare March 11, 2024 11:16

polydez mentioned this pull request Mar 11, 2024

Make ProvenTransaction::initial_account_hash optional #522

Closed

bobbinth approved these changes Mar 11, 2024

View reviewed changes

objects/Cargo.toml Outdated Show resolved Hide resolved

objects/src/transaction/proven_tx.rs Show resolved Hide resolved

bobbinth mentioned this pull request Mar 12, 2024

Simplify AccountDetails in ProvenTransaction #524

Closed

docs: added changelog

68b0afb

hackaugusto force-pushed the hacka-issue-403-2 branch from 30cdf8c to 68b0afb Compare March 12, 2024 15:34

hackaugusto merged commit 6bcefdc into next Mar 12, 2024
8 checks passed

hackaugusto deleted the hacka-issue-403-2 branch March 12, 2024 15:42

polydez mentioned this pull request Mar 13, 2024

Implement ProvenTransaction validation #528

Closed

bobbinth mentioned this pull request Apr 11, 2024

On-chain accounts and notes #403

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account on-chain tracking #485

Account on-chain tracking #485

hackaugusto commented Feb 27, 2024 •

edited

Loading

hackaugusto Feb 28, 2024 •

edited

Loading

hackaugusto commented Mar 4, 2024

bobbinth left a comment

hackaugusto commented Mar 5, 2024

hackaugusto commented Mar 5, 2024

hackaugusto commented Mar 6, 2024

hackaugusto commented Mar 8, 2024

bobbinth left a comment

bobbinth commented Mar 8, 2024

hackaugusto commented Mar 11, 2024

bobbinth left a comment

Account on-chain tracking #485

Account on-chain tracking #485

Conversation

hackaugusto commented Feb 27, 2024 • edited Loading

hackaugusto Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

hackaugusto commented Mar 4, 2024

bobbinth left a comment

Choose a reason for hiding this comment

hackaugusto commented Mar 5, 2024

hackaugusto commented Mar 5, 2024

hackaugusto commented Mar 6, 2024

hackaugusto commented Mar 8, 2024

bobbinth left a comment

Choose a reason for hiding this comment

bobbinth commented Mar 8, 2024

hackaugusto commented Mar 11, 2024

bobbinth left a comment

Choose a reason for hiding this comment

hackaugusto commented Feb 27, 2024 •

edited

Loading

hackaugusto Feb 28, 2024 •

edited

Loading