Skip to content

Conversation

@JoseSK999
Copy link
Contributor

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other:

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other: .

Description

In order to test process_proof without having a ChainState with the previous blocks, we make the function take a generic F: Fn(u32) -> Result<BlockHash, E>.

Then for tests we pass a closure with the required hashes, and elsewhere we use |h| self.chain.get_block_hash(h), instead of passing the whole &self.chain.

We also move the udata.proof conversion into the From trait implementation. Finally reduce nesting in the process_proof body.

Notes to the reviewers

The behavior of process_proof should be unchanged, I only refactored the BatchProof conversion to Proof, renamed inputs to utxos, hashes to del_hashes, and reduced nesting by calling continue.

In order to test `process_proof` without having a `ChainState` with the previous blocks, we make the function take a generic `F: Fn(u32) -> Result<BlockHash, E>`.

Then for tests we pass a closure with the required hashes, and elsewhere we use `|h| self.chain.get_block_hash(h)`, instead of passing the whole `&self.chain`.

We also move the `udata.proof` conversion into the `From` trait implementation. Finally reduce nesting in the `process_proof` body.
@JoseSK999 JoseSK999 force-pushed the process-proof-test branch from ecb64a7 to 5722817 Compare March 20, 2025 17:47
@JoseSK999
Copy link
Contributor Author

Done! I have added the trait bound E: From<Error> to process_proof in order to propagate the reconstruct_leaf_data error, rather than panicking.

Then implemented From<Error> for BlockchainError by returning the UtreexoError variant, which seems the correct kind.

Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

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

ACK 5722817

@Davidson-Souza Davidson-Souza merged commit 2d39828 into vinteumorg:master Mar 22, 2025
8 checks passed
@JoseSK999 JoseSK999 deleted the process-proof-test branch March 22, 2025 22:23
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