Skip to content

Conversation

@MartinOndejka
Copy link
Collaborator

@MartinOndejka MartinOndejka commented Sep 4, 2025

Fixes of #345 review

Copy link
Collaborator

@L-as L-as left a comment

Choose a reason for hiding this comment

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

The ledger issue hasn't been fixed, it should still be a signature of ledger and acc set.

Consider:
Rollup sends information to DA, which stores the data necessary to get the data for the ledger, and the acc set derived from that, and signs the ledger.

But the rollup can in fact locally use a different acc set crafted differently.

Unless I'm wrong.

Field.Checked.equal empty_hash hash_other
>>= Boolean.( &&& ) is_right >>| Boolean.not )
in
Boolean.all checks
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should probably be a comment explaining the logic.
Whenever we replace an empty account with a non-empty account, it must be that all accounts before it must be non-empty.
We can define this inductively.
For the depth 0 ledger of one account, the condition always holds.
For the depth 1 ledger of two accounts, we can only fill the right account if the left account isn't the empty account.
For the depth 2 ledger of four accounts, we can only fill, if the account is in the left subtree, then the same condition as above, otherwise it's the same condition as above but also the left subtree must not have any empty accounts.

Consider then your above code.

If only index (left, left) is non-empty,
and we then insert into (right, left) instead of (left, right),
I think the checks would all succeed, because it's not empty hash for the first level,
and it's left for the second level.

Also you could reformulate it as empty_hash != hash_other || is_left to make it simpler.

You probably need another field in the outer account to keep track of the right-most account,
such that if you make a new account, then the index must be last + 1, and then update it appropriately.

The above code checks that the trees on right are all empty,
but it doesn't correctly check that there are no empty subtrees on the left.

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.

3 participants