Fix multiple deposits #126
Conversation
vaibhavchellani
force-pushed
the
fix-multiple-deposits
branch
from
35aaf89
to
01f02c0
Compare
| err = DBI.InitAccountTree(int(accountTreeDepth.Uint64())) | ||
| err = DBI.InitAccountTree(int(accountTreeDepth.Uint64()) + 1) |
There was a problem hiding this comment.
Nice catch here! The DEPTH in the accountTree contract is misleading, created an issue to fix it later https://github.com/thehubbleproject/hubble-contracts/issues/457😓 .
There was a problem hiding this comment.
This one lead to finding another big issue, so no harm no foul.
| ) | ||
|
|
||
| // Aggregator is the service which is supposed to create batches | ||
| // It has the following tasks: | ||
| // 1. Pick txs from the mempool | ||
| // 1. Pick txs from the mempoolinaccurate number of transactione |
There was a problem hiding this comment.
Is this accidentally committed?
There was a problem hiding this comment.
yeah, will change
| func encodePacked(input ...[]byte) []byte { | ||
| return bytes.Join(input, nil) | ||
| } |
There was a problem hiding this comment.
Great work solving this!
| @@ -17,7 +17,7 @@ func init() { | |||
| panic(err) | |||
| } | |||
|
|
|||
| DefaultHashes, err = GenDefaultHashes(100) | |||
| DefaultHashes, err = GenDefaultHashes(100, ZeroLeaf) | |||
There was a problem hiding this comment.
No change is required here. Just want to point out that we call the zero/empty leaf differently between the contract and the client.
thehubbleproject/hubble-contracts#376 (comment)
| if len(leaves)%2 != 0 { | ||
| leaves = append(leaves, ZeroLeaf) | ||
| } |
There was a problem hiding this comment.
Here we handled the odd number of leaves only in the bottom level. But the odd number of nodes should be handled at every level.
For example, say we have 9 leaves to merklise. This makes a 4 level tree (not counting the root).
- We add defaultHash[0] in level 0 leaves. After the ascend, level 1 has 5 nodes
- We add defaultHash[1] in level 1 nodes to make them 6 nodes. After the ascend, level 2 has 3 nodes.
- We add defaultHash[2] in level 2 nodes to make them 4 nodes. After the ascend, level 3 has 2 nodes.
I suggest this logic is better handled in the ascend method.
| if len(txs) < int(a.cfg.TxsPerCommitment) { | ||
| return commitments, ErrNotEnoughTransactions | ||
| } |
There was a problem hiding this comment.
Does a transaction take a long time to be submitted on chain if the commitment is unfilled?
There was a problem hiding this comment.
not really, it just might be non economical to submit unfilled commitments on-chain for a coordinator, so we allow him to configure that tightly.
No description provided.