Merge pull request #15267 from MinaProtocol/fix-coinbase-witness

Fix order of accounts in ledger coinbase witness

src/lib/mina_base/coinbase.ml

@@ -56,7 +56,13 @@ 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 as the tx
+       application (in `apply_coinbase`) 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
 
   let accounts_referenced t =
     List.map (account_access_statuses t Transaction_status.Applied)

src/lib/mina_ledger/ledger.mli

@@ -167,7 +167,8 @@ val gen_initial_ledger_state : init_state Quickcheck.Generator.t
 (** Apply a generated state to a blank, concrete ledger. *)
 val apply_initial_ledger_state : t -> init_state -> unit
 
-module Ledger_inner : Ledger_intf.S with type t = t
+module Ledger_inner :
+  Ledger_intf.S with type t = t and type location = Location.t
 
 module For_tests : sig
   open Currency

src/lib/mina_ledger/test/helpers/ledger_helpers.ml

@@ -16,11 +16,19 @@ let bin_log n =
   in
   find 0
 
-let ledger_of_accounts accounts =
+type ledger_depth = Fixed_depth of int | Inferred_depth
+
+let ledger_of_accounts ?(depth = Inferred_depth) accounts =
   let open Result.Let_syntax in
   let open Test_account in
   let module R = Monad_lib.Make_ext2 (Result) in
-  let depth = bin_log @@ List.length accounts in
+  let depth =
+    match depth with
+    | Inferred_depth ->
+        bin_log @@ List.length accounts
+    | Fixed_depth d ->
+        d
+  in
   let ledger = Ledger.empty ~depth () in
   let%map () =
     R.iter_m accounts ~f:(fun a ->

src/lib/transaction_logic/test/transaction_logic/main.ml

@@ -7,5 +7,17 @@ let () =
         , [ test_case "Simple funds transfer" `Quick simple_payment
           ; test_case "Fee payer cannot be different than sender" `Quick
               simple_payment_signer_different_from_fee_payer
+          ; test_case
+              "Coinbase transaction creates accounts in the correct order (no \
+               fee transfer)"
+              `Quick
+              (coinbase_order_of_created_accounts_is_correct
+                 ~with_fee_transfer:false )
+          ; test_case
+              "Coinbase transaction creates accounts in the correct order \
+               (with fee transfer)"
+              `Quick
+              (coinbase_order_of_created_accounts_is_correct
+                 ~with_fee_transfer:true )
           ] )
     ]

src/lib/transaction_logic/test/transaction_logic/transaction_logic.ml

@@ -115,3 +115,44 @@ let simple_payment_signer_different_from_fee_payer () =
           "Cannot pay fees from a public key that did not sign the transaction"
         (Transaction_logic.apply_transactions ~constraint_constants ~global_slot
            ~txn_state_view ledger [ txn ] ) )
+
+let coinbase_order_of_created_accounts_is_correct ~with_fee_transfer () =
+  let amount = Amount.of_mina_int_exn 720 in
+  let make_nondeterministic_pk () =
+    Private_key.create () |> Public_key.of_private_key_exn
+    |> Public_key.compress
+  in
+  let receiver = make_nondeterministic_pk () in
+  let fee_transfer =
+    if with_fee_transfer then
+      let receiver_pk = make_nondeterministic_pk () in
+      let fee = Fee.of_mina_int_exn 10 in
+      Some (Coinbase_fee_transfer.create ~receiver_pk ~fee)
+    else None
+  in
+  let coinbase_txn =
+    Or_error.ok_exn @@ Coinbase.create ~amount ~receiver ~fee_transfer
+  in
+  let accounts = [] (* All accounts are new *) in
+  let ledger =
+    match Ledger_helpers.ledger_of_accounts ~depth:(Fixed_depth 2) accounts with
+    | Ok l ->
+        l
+    | Error _ ->
+        assert false
+  in
+  let txn_global_slot = Global_slot_since_genesis.of_int 5 in
+  [%test_pred: Transaction_applied.Coinbase_applied.t Or_error.t] Or_error.is_ok
+    (Transaction_logic.apply_coinbase ~constraint_constants ~txn_global_slot
+       ledger coinbase_txn ) ;
+  let int_loc_of_account pk =
+    Ledger.location_of_account ledger pk
+    |> Option.value_exn |> Mina_ledger.Ledger.Location.to_path_exn
+    |> Mina_ledger.Ledger.Addr.to_int
+  in
+  let coinbase_accounts_referenced =
+    Coinbase.accounts_referenced coinbase_txn
+  in
+  List.iteri coinbase_accounts_referenced ~f:(fun idx pk ->
+      let actual_idx = int_loc_of_account pk in
+      [%test_eq: int] actual_idx idx )