Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix order of accounts in ledger coinbase witness #15267

Merged
merged 5 commits into from Mar 4, 2024
Merged

Fix order of accounts in ledger coinbase witness #15267

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
79b5963
Fix order of new accounts in the ledger witness, on coinbase tx
sebastiencs Aug 14, 2023
673faf8
Add a test for re-ordered `Coinbase.accounts_referenced`
mrmr1993 Mar 4, 2024
12cabeb
Fixup comment
mrmr1993 Mar 4, 2024
437d0a3
Use labelled variants instead of `Option` for ledger depth
mrmr1993 Mar 4, 2024
60cd88c
Merge branch 'berkeley' into fix-coinbase-witness
mrmr1993 Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/mina_base/coinbase.ml
View file
Open in desktop
Expand Up @@ -56,7 +56,11 @@ module Make_str (A : Wire_types.Concrete) = struct
receiver t
:: List.map ~f:Fee_transfer.receiver (Option.to_list t.fee_transfer)
in
(* The order of this list will impact the order of new accounts in the ledger witness.
We use `List.rev` to have the same order than the tx application (in `apply_coinbase`)
mrmr1993 marked this conversation as resolved.
Show resolved Hide resolved
where the "coinbase fee transfer" receiver is created before the "coinbase" receiver *)
List.map account_ids ~f:(fun acct_id -> (acct_id, access_status))
|> List.rev
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this whole piece after let access_status definition can be simplified as something like:

let fee_transfer =
  let%map.Option transfer = t.fee_transfer in
  (Fee_transfer.receiver transfer, access_status)
in
Option.value_map ~default:ident ~f:List.cons fee_transfer [(receiver t, access_status)]

Current code is a bit hard to read
CC @mrmr1993


let accounts_referenced t =
List.map (account_access_statuses t Transaction_status.Applied)
Expand Down