Skip to content

refactor: prover logic, dependency updates#28

Merged
ananas-block merged 6 commits intojorrit/feat-add-testfrom
sergey/cleanup-deps
May 18, 2025
Merged

refactor: prover logic, dependency updates#28
ananas-block merged 6 commits intojorrit/feat-add-testfrom
sergey/cleanup-deps

Conversation

@sergeytimoshin
Copy link

@sergeytimoshin sergeytimoshin commented May 16, 2025

1. Dependency updates

  • Updated light-concurrent-merkle-tree, light-batched-merkle-tree, light-merkle-tree-metadata, light-compressed-account, light-hasher, light-merkle-tree-reference) to a newer revision (a160153... from 368f9f0...).
  • Removed light-sdk.

2. Public Transaction Events

3. Prover client

  • Proof Component Types: The ProofABC struct now uses fixed-size arrays, providing stricter type definitions.
  • Proof Compression and Negation:
  • Replaced the local change_endianness function and refined G1 point negation in gnark.rs. The new negate_g1 function now uses convert_endianness from solana_program.
  • Introduced a compress_proof function that utilizes alt_bn128_g1_compress and alt_bn128_g2_compress for standardized proof compression, replacing the previous negate_proof logic.
  • Logic Adjustment: The root_seq assignment in has been adjusted to be conditional based on tree_height.

4. Error Handling

@sergeytimoshin sergeytimoshin marked this pull request as draft May 16, 2025 12:42
@sergeytimoshin sergeytimoshin marked this pull request as ready for review May 16, 2025 18:16
@sergeytimoshin sergeytimoshin changed the title refactor: cleanup deps refactor: prover logic, dependency updates May 17, 2025
PhotonApiError::UnexpectedError(format!("Failed to serialize pubkey: {}", e))
})?,
root_seq: 3,
root_seq: if TREE_HEIGHT_V1 == tree_height { 3 } else { 0 },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks suspicious. Are you sure this works once the tree is being forested?

Copy link

@ananas-block ananas-block May 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intuitions is that it should be something like:
root_seq: if TREE_HEIGHT_V1 == tree_height { (seq % 2400) } else { (seq % 20) },

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for the case when tree is empty:

For non-empty trees we use

let root_seq = match node_to_model.get(&(leaf_node.tree.to_bytes_vec(), 1)) {
Some(root) => root.seq,
None => None,
};

Copy link

@ananas-block ananas-block May 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we don't need to do this:

for elem in proof.iter() {
            root = compute_parent_hash(root, elem.to_vec()).map_err(|e| {
                PhotonApiError::UnexpectedError(format!("Failed to compute hash: {}", e))
            })?;
        }

We can just do:
ZERO_BYTES[tree_height]

and there is no need to validate the Merkle proof since it's just zero values.
We can remove:
merkle_proof.validate()?;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wdyt about extracting this branch if btree.is_empty() { into a function so that the empty case is not ambiguous?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we don't need to do this:

for elem in proof.iter() {
            root = compute_parent_hash(root, elem.to_vec()).map_err(|e| {
                PhotonApiError::UnexpectedError(format!("Failed to compute hash: {}", e))
            })?;
        }

We can just do: ZERO_BYTES[tree_height]

We can't do that, because the zeroeth_element_hash is not ZERO_BYTES[0]. When btree.is_empty(), we are constructing a proof for the zeroeth_element.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wdyt about extracting this branch if btree.is_empty() { into a function so that the empty case is not ambiguous?

Sure https://github.com/Lightprotocol/photon/blob/sergey/cleanup-deps/src/ingester/persist/persisted_indexed_merkle_tree.rs#L135

@ananas-block ananas-block merged commit 2800b7f into jorrit/feat-add-test May 18, 2025
@ananas-block ananas-block deleted the sergey/cleanup-deps branch May 18, 2025 20:15
sergeytimoshin added a commit that referenced this pull request May 30, 2025
* chore: update dependencies and clean up unused packages

* refactor: consolidate PublicTransactionEvent structs and update references

* refactor: adjust root_seq assignment and add Debug trait to struct definitions

* refactor: proof compression and error handling in prover

* refactor: simplify AddressProofInputs, rootIndex: u16

* refactor: extract proof generation for empty tree into separate function
sergeytimoshin added a commit that referenced this pull request Jun 8, 2025
* refactor: add support for batched updates

* refactor: add support for batched updates

Update src/dao/generated/accounts.rs

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

Update src/ingester/parser/batch_event_parser.rs

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

refactor: remove getSubtrees method and related API documentation

Update src/ingester/parser/mod.rs

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

Refactor struct fields to use camelCase naming convention

replace the `calculate_two_inputs_hash_chain` function with the `create_two_inputs_hash_chain` method from the `light_compressed_account` crate.

Refactor error handling in `parse_public_transaction_event_v2`

format

Remove obsolete and commented-out account update code

Move `node_index_to_leaf_index` function to the appropriate location and remove dead code

Add comments to clarify nullifier field usage

Add comments to clarify tx_hash field usage in account struct

Add comments to clarify seq field usage in account struct

Add comments to clarify nullifier_queue_index field usage in account struct

Refactor get_compressed_accounts_by_owner module and add common utilities for account filtering

add validity proof v2

Add get_validity_proof_v2 and update API specifications

remove unnecessary logging

fix: mock tests

fix for getValidityProof (v1)

refactor: remove unused tree height parameters and add getValidityProofV2 method

fix: update method name from address_from_bytes to state_from_bytes in mod.rs consistency.

fix indexed_accounts query

fix get_compressed_account_by_owner v1

refactor: simplify account handling and improve code consistency in transaction processing

refactor: remove queue_position handling and update related queries and indexes

upd .gitignore

cargo fmt and fixed tests

* fix mock test

* refactor: validate heights of inclusion and non-inclusion proofs

* refactor: implement AccountWithContext constructor and remove parse_account_data function

* refactor: move spend_input_accounts_batched function to spend_batch module

* refactor: rename spend_batch module to spend and move spend_input_accounts function

* refactor: streamline transaction parsing

* refactor: restructure ingester/parser module

* refactor: restructure ingester/persist module

* add v2 endpoints

wip e2e test

make get_compressed_accounts_by_owner_v2 return AccountV2 primitive

add mock_tests.rs

add prove_by_index to AccountV2

refactor: validate heights of inclusion and non-inclusion proofs

refactor: implement AccountWithContext constructor and remove parse_account_data function

refactor: move spend_input_accounts_batched function to spend_batch module

refactor: rename spend_batch module to spend and move spend_input_accounts function

refactor: streamline transaction parsing

refactor: restructure ingester/parser module

refactor: restructure ingester/persist module

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

test: add compressed token in batched tree test

* chore: renamed tests, wip reference Merkle tree to assert root

* test: correct root

* test: add get_queue_elements test

* refactor: get_validity_proof_v2 and add tests

* removed leftover print

* feat: add v2 endpoints and update response schemas for compressed accounts and transactions

* fix: tests

* fix: get_queue_elements response ordering

* fix: spend_input_accounts_batched

* parser cleanup

* refactor: persist batch events

* chore: add get_queue_elements test

* fix: flaky nullification of batched input accounts

* updated dep commit

* refactor: update queue size calculation for batch inputs based on state tree height

* feat: enhance account structure with tree type and update queue handling

* refactor: fix SQL syntax in account initialization and update queue assertion

* refactor: correct queue access in batched state tree tests and adjust instruction index check

* feat: update AccountContext to include tree type in mock tests and utils

* tests wip

* feat: add queue and tree_type fields to mock test data

* chore: update light-protocol dependencies

* refactor: update light-protocol dependencies and improve error handling in API functions

* cleanup

* feat: update API responses to include V2 schemas and add new fields for compression info

* chore: rename GetCompressedAccountsByOwnerV2Response -> GetCompressedAccountsByOwnerResponseV2

* chore: updated to refactored event_from_light_transaction

* test: four cpi events

* * add validation for maximum allowed hashes in get_validity_proof_v2 function
* sort accounts by the same order as request.hashes

* feat: refactor account structures to use MerkleContextV2 and update API schemas

* pass as u16

* rm

* refactor: remove constant for maximum allowed hashes in get_validity_proof_v2 function

* feat: move account*, context and limit structs to typedefs

* fix: update parse_leaf_index function to accept i64 and adjust related usages

* feat: move TryFrom for AccountWithContext to account/context.rs

* refactor: remove parse_discriminator function from utils.rs

* refactored event to use unified enum doesn't work

* add new test data

* fix: refactor

* * implement v2 API for compressed account proofs
* refactored TreeAndQueue and tree heights helpers

* Update src/api/method/get_compressed_account_proof/v2.rs

Co-authored-by: ananas-block <58553958+ananas-block@users.noreply.github.com>

* fix: add error handling for mismatched accounts and hashes in v2 API

* fix: add error handling for missing account in v2 API

* feat: integrate ContextInfo into GetCompressedAccountProofResponseValueV2

* refactor: clean up imports in tx_event_parser.rs

* refactor: remove debug print statement from leaf_node.rs

* refactor: simplify GetMultipleCompressedAccountProofsResponseValueV2 structure and update related parsing logic

* refactor: simplify GetMultipleCompressedAccountProofsResponseV2 by removing nested structure

* feat: add v2 endpoints for compressed account proofs and update API specifications

* refactor: remove debug print statements and clean up imports in API modules

* consistent tree field in v2 responses

* validityproofv2 merkle_context -> merkle_contexts

* update naming for get_transaction_with_compression_info helpers v2

* fix batch events processing

cleanup

cleanup

* remove unused AccountContext

* chore: remove unused StateUpdate field

* move parse_token_data_v2 to get_transaction_with_compression_info

* chore: add test description

* wip

* register getCompressedAccountProofV2

* fix: update test assertions

* fix: GetQueueElementsResponseValue camelCase

* feat: batch address update support

* feat: implement batch address tree tests

* refactor: remove debug print statements from multiple modules

* fix: update .gitignore to ignore all .env files

* fix: use create_dir_all for creating temp directory

* feat: add legacy state and address trees to tree_info

* feat: implement conversion from PublicTransactionEventV1 and V2 to PublicTransactionEvent

* refactor: remove PublicTransactionEvent enum and related conversions

* update tests data

* update tests data

* remove leftover

* remove leftover

* remove leftover

* update batched address tree tests

* batched address tree tests wip

* cleanup

* cleanup

* cleanup

* cleanup

* add support for custom account compression program ID

* * For get_multiple_new_address_proofs if request tree is V2, requested address is in the AddressQueue but not in tree yet, we should return error. For V1 trees we still return non-inclusion proof, because we don't have information about V1 queue.
* Add offset as request parameter to get_batch_address_update_info.
* Various cleanups

* Switch to `solana-pubkey` and update dependencies.

* Switch to `solana-pubkey` and update dependencies.

* Update import for Pubkey from solana_pubkey and adjust account key mapping

* update tree height handling and clean up debug prints

* fix test environment comment and increment state tree height for leaf node persistence

* test tx data

* test tx data

* test tx data

* test tx data

* refactor address tree handling and improve logging for batch updates

* refactor batched address tests

* Move `light-merkle-tree-reference` to dev-dependencies

* fix: test batched tree transactions test (#24)

* refactor: validity proof v2 response layout

* refactor: update dependencies and improve proof handling

* 1. updated proof deserialization to skip validation for performance improvements by setting `Validate::No`.
2. removed commented-out unused variables in the prover logic.

* updated `negate_proof` to return `Result` for better error handling

* chore: update ark-* to 0.5

* refactor: remove commented-out code for clarity in v2.rs

* refactor: prover logic, dependency updates (#28)

* chore: update dependencies and clean up unused packages

* refactor: consolidate PublicTransactionEvent structs and update references

* refactor: adjust root_seq assignment and add Debug trait to struct definitions

* refactor: proof compression and error handling in prover

* refactor: simplify AddressProofInputs, rootIndex: u16

* refactor: extract proof generation for empty tree into separate function

* refactor: improve transaction parsing logic and clean up code

* Update prover docker image to sergeytimoshin/prover:latest

* Export proof input types for OpenAPI schema generation

* chore: remove tree.txt, replace git dependencies with released crates

* chore: replace git dev dep with released dep

* chore: rename start_offset -> start_queue_index, zkp_batch_size -> limit, num_elements -> limit

---------

Co-authored-by: Sergey Timoshin <sergeytimoshin@proton.me>
Co-authored-by: Swenschaeferjohann <swen@lightprotocol.com>
Co-authored-by: Sergey Timoshin <timoshin.sergey@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants