Conversation
* Bug fixes - Refresh poolClinetPublicKey when we switch wallets - Store notes per wallet address * Fixed the error message when proof fails
…ansfers # Conflicts: # app/js/ui.js
| next_index: u64, | ||
| } | ||
|
|
||
| // TODO: For now we implement a full merkle tree. We should study if a partial |
There was a problem hiding this comment.
We should remove the TODO if we solve it
| if index_u64 > self.next_index { | ||
| return Err(format!( | ||
| "insert_at: index {} exceeds next_index {}, would create gap", | ||
| index, self.next_index | ||
| )); | ||
| } |
There was a problem hiding this comment.
I don't think this is what we want. The idea of having insertAt is to allow these gaps to happen, and to be solved when receiving missing leaves later.
E.g., If a user receives leaf 8 5ms before leaf 7, it does not panic, and after 5 ms the root will be the same as if we received leaves in order.
If something fails, besides reasonable delays, the root won't match, and the proof will fail. But that will be caused by other issues (e.g. RPC failures).
| let parent_level = level | ||
| .checked_add(1) | ||
| .ok_or_else(|| JsValue::from_str("Level overflow"))?; | ||
| let parent_level = level.checked_add(1).expect("level < depth <= 32"); |
There was a problem hiding this comment.
Let's use Result as return value instead of expect
| tree.insert(&leaf(1)).expect("insert 0"); | ||
| tree.insert(&leaf(2)).expect("insert 1"); | ||
| tree.insert(&leaf(3)).expect("insert 2"); |
There was a problem hiding this comment.
A bit of nitpicking. Expect works for tests, but error messages could be more detailed
| } | ||
|
|
||
|
|
||
| await persistTree(); |
There was a problem hiding this comment.
This works. But it means we serialize and save the tree after each processed leaf. Not sure if it could be batched to make fewer allocations. Can be addressed on a different PR. Feel free to ignore.
There was a problem hiding this comment.
This also happens in pool-store.js
| if (!poolStore.isTreeInitialized()) { | ||
| await poolStore.rebuildTree(); | ||
| } |
There was a problem hiding this comment.
This assumes that the tree will be rebuilt only if it is null. Meaning that we assume the restored version will be up to date. If the restored version is missing any leaves for any reason, it will fail.
I did try to run a small demo (see https://github.com/NethermindEth/stellar-private-transactions?tab=readme-ov-file#demo-application), but the deposit failed. Probably because there's a mismatch between the off-chain and on-chain roots.
That was an error on my end, due to a mixed state from a previous deployment. The demo still works. So it seems the assumption is correct so far 🙌
Fantoni0
left a comment
Fantoni0
left a comment
There was a problem hiding this comment.
The idea is good 🙌 , but some changes can be made. And we need to ensure it does note break the demo functionality.
3 participants
Summary
insert_at(leaf, index)to MerkleTree for targeted insertion, replacing a 1024-iteration insert loop in transaction-builder with a single callserialize()/deserialize()for MerkleTree with IndexedDBtree_snapshotsstore, so page reloads restore the tree from a snapshot instead of replaying all leavesbuild_from_leaves()batch constructor that builds the tree in a single bottom-up pass, used when rebuilding from > 100 stored leaves