Alpha: better error for invalid key revelation

src/proto_alpha/lib_protocol/src/alpha_context.mli

@@ -438,6 +438,9 @@ module Contract : sig
   val exists: context -> contract -> bool tzresult Lwt.t
   val must_exist: context -> contract -> unit tzresult Lwt.t
 
+  val allocated: context -> contract -> bool tzresult Lwt.t
+  val must_be_allocated: context -> contract -> unit tzresult Lwt.t
+
   val list: context -> contract list Lwt.t
 
   type origination_nonce
@@ -450,8 +453,12 @@ module Contract : sig
 
   val get_manager:
     context -> contract -> public_key_hash tzresult Lwt.t
-  val update_manager_key:
-    context -> contract -> public_key option -> (context * public_key) tzresult Lwt.t
+
+  val get_manager_key:
+    context -> contract -> public_key tzresult Lwt.t
+
+  val reveal_manager_key:
+    context -> contract -> public_key -> context tzresult Lwt.t
 
   val is_delegatable:
     context -> contract -> bool tzresult Lwt.t

src/proto_alpha/lib_protocol/src/apply.ml

@@ -38,6 +38,7 @@ type error += Outdated_double_baking_evidence
   of { level: Raw_level.t ; last: Raw_level.t } (* `Permanent *)
 type error += Invalid_activation of { pkh : Ed25519.Public_key_hash.t }
 type error += Wrong_activation_secret
+type error += Multiple_revelation
 
 let () =
   register_error_kind
@@ -313,7 +314,18 @@ let () =
         Format.fprintf ppf "Wrong activation secret.")
     Data_encoding.unit
     (function Wrong_activation_secret -> Some () | _ -> None)
-    (fun () -> Wrong_activation_secret)
+    (fun () -> Wrong_activation_secret) ;
+  register_error_kind
+    `Permanent
+    ~id:"block.multiple_revelation"
+    ~title:"Multiple revelations were included in a manager operation"
+    ~description:"A manager operation should not contain more than one revelation"
+    ~pp:(fun ppf () ->
+        Format.fprintf ppf
+          "Multiple revelations were included in a manager operation")
+    Data_encoding.empty
+    (function Multiple_revelation -> Some () | _ -> None)
+    (fun () -> Multiple_revelation)
 
 let apply_consensus_operation_content ctxt
     pred_block block_priority operation = function
@@ -446,15 +458,23 @@ let apply_sourced_operation
     operation origination_nonce ops =
   match ops with
   | Manager_operations { source ; fee ; counter ; operations = contents } ->
-      let public_key =
+      let revealed_public_keys =
         List.fold_left (fun acc op ->
             match op with
-            | Reveal pk -> Some pk
-            | _ -> acc) None contents in
-      Contract.must_exist ctxt source >>=? fun () ->
-      Contract.update_manager_key ctxt source public_key >>=? fun (ctxt,public_key) ->
-      Operation.check_signature public_key operation >>=? fun () ->
+            | Reveal pk -> pk :: acc
+            | _ -> acc) [] contents in
+      Contract.must_be_allocated ctxt source >>=? fun () ->
       Contract.check_counter_increment ctxt source counter >>=? fun () ->
+      begin
+        match revealed_public_keys with
+        | [] -> return ctxt
+        | [pk] ->
+            Contract.reveal_manager_key ctxt source pk
+        | _ :: _ :: _ ->
+            fail Multiple_revelation
+      end >>=? fun ctxt ->
+      Contract.get_manager_key ctxt source >>=? fun public_key ->
+      Operation.check_signature public_key operation >>=? fun () ->
       Contract.increment_counter ctxt source >>=? fun ctxt ->
       Contract.spend ctxt source fee >>=? fun ctxt ->
       add_fees ctxt fee >>=? fun ctxt ->

src/proto_alpha/lib_protocol/src/bootstrap_storage.ml

@@ -11,8 +11,7 @@ let init ctxt (account: Parameters_repr.bootstrap_account) =
   let public_key_hash = Signature.Public_key.hash account.public_key in
   let contract = Contract_repr.implicit_contract public_key_hash in
   Contract_storage.credit ctxt contract account.amount >>=? fun ctxt ->
-  Contract_storage.update_manager_key ctxt contract
-    (Some account.public_key) >>=? fun (ctxt, _) ->
+  Contract_storage.reveal_manager_key ctxt contract account.public_key >>=? fun ctxt ->
   Delegate_storage.set ctxt contract (Some public_key_hash) >>=? fun ctxt ->
   return ctxt
 

src/proto_alpha/lib_protocol/src/contract_storage.ml

@@ -13,10 +13,13 @@ type error +=
   | Counter_in_the_future of Contract_repr.contract * int32 * int32 (* `Temporary *)
   | Unspendable_contract of Contract_repr.contract (* `Permanent *)
   | Non_existing_contract of Contract_repr.contract (* `Temporary *)
+  | Empty_implicit_contract of Signature.Public_key_hash.t (* `Temporary *)
   | Inconsistent_hash of Signature.Public_key.t * Signature.Public_key_hash.t * Signature.Public_key_hash.t (* `Permanent *)
   | Inconsistent_public_key of Signature.Public_key.t * Signature.Public_key.t (* `Permanent *)
   | Missing_public_key of Signature.Public_key_hash.t (* `Permanent *)
   | Failure of string (* `Permanent *)
+  | Previously_revealed_key of Contract_repr.t (* `Permanent *)
+  | Unrevealed_manager_key of Contract_repr.t (* `Permanent *)
 
 let () =
   register_error_kind
@@ -137,7 +140,44 @@ let () =
     ~pp:(fun ppf s -> Format.fprintf ppf "Contract_storage.Failure %S" s)
     Data_encoding.(obj1 (req "message" string))
     (function Failure s -> Some s | _ -> None)
-    (fun s -> Failure s)
+    (fun s -> Failure s) ;
+  register_error_kind
+    `Branch
+    ~id:"contract.unrevealed_key"
+    ~title:"Manager operation precedes key revelation"
+    ~description:
+      "One tried to apply a manager operation \
+       without revealing the manager public key"
+    ~pp:(fun ppf s ->
+        Format.fprintf ppf "Unrevealed manager key for contract %a."
+          Contract_repr.pp s)
+    Data_encoding.(obj1 (req "contract" Contract_repr.encoding))
+    (function Unrevealed_manager_key s -> Some s | _ -> None)
+    (fun s -> Unrevealed_manager_key s) ;
+  register_error_kind
+    `Branch
+    ~id:"contract.previously_revealed_key"
+    ~title:"Manager operation already revealed"
+    ~description:
+      "One tried to revealed twice a manager public key"
+    ~pp:(fun ppf s ->
+        Format.fprintf ppf "Previously revealed manager key for contract %a."
+          Contract_repr.pp s)
+    Data_encoding.(obj1 (req "contract" Contract_repr.encoding))
+    (function Previously_revealed_key s -> Some s | _ -> None)
+    (fun s -> Previously_revealed_key s) ;
+  register_error_kind
+    `Branch
+    ~id:"implicit.empty_implicit_contract"
+    ~title:"Empty implicit contract"
+    ~description:"No manager operations are allowed on an empty implicit contract."
+    ~pp:(fun ppf implicit ->
+        Format.fprintf ppf
+          "Empty implicit contract (%a)"
+          Signature.Public_key_hash.pp implicit)
+    Data_encoding.(obj1 (req "implicit" Signature.Public_key_hash.encoding))
+    (function Empty_implicit_contract c -> Some c | _ -> None)
+    (fun c -> Empty_implicit_contract c)
 
 let failwith msg = fail (Failure msg)
 
@@ -191,19 +231,29 @@ let delete c contract =
   Storage.Contract.Big_map.clear (c, contract) >>= fun c ->
   return c
 
+let allocated c contract =
+  Storage.Contract.Counter.get_option c contract >>=? function
+  | None -> return false
+  | Some _ -> return true
+
 let exists c contract =
   match Contract_repr.is_implicit contract with
   | Some _ -> return true
-  | None ->
-      Storage.Contract.Counter.get_option c contract >>=? function
-      | None -> return false
-      | Some _ -> return true
+  | None -> allocated c contract
 
 let must_exist c contract =
   exists c contract >>=? function
   | true -> return ()
   | false -> fail (Non_existing_contract contract)
 
+let must_be_allocated c contract =
+  allocated c contract >>=? function
+  | true -> return ()
+  | false ->
+      match Contract_repr.is_implicit contract with
+      | Some pkh -> fail (Empty_implicit_contract pkh)
+      | None -> fail (Non_existing_contract contract)
+
 let list c = Storage.Contract.list c
 
 let check_counter_increment c contract counter =
@@ -251,25 +301,22 @@ let get_manager c contract =
   | Some (Manager_repr.Hash v) -> return v
   | Some (Manager_repr.Public_key v) -> return (Signature.Public_key.hash v)
 
-let update_manager_key c contract = function
-  | Some public_key ->
-      begin Storage.Contract.Manager.get c contract >>=? function
-        | Public_key v -> (* key revealed for the second time *)
-            if Signature.Public_key.(v = public_key) then return (c,v)
-            else fail (Inconsistent_public_key (v,public_key))
-        | Hash v ->
-            let actual_hash = Signature.Public_key.hash public_key in
-            if (Signature.Public_key_hash.equal actual_hash v) then
-              let v = (Manager_repr.Public_key public_key) in
-              Storage.Contract.Manager.set c contract v >>=? fun c ->
-              return (c,public_key) (* reveal and update key *)
-            else fail (Inconsistent_hash (public_key,v,actual_hash))
-      end
-  | None ->
-      begin Storage.Contract.Manager.get c contract >>=? function
-        | Public_key v -> return (c,v) (* already revealed *)
-        | Hash v -> fail (Missing_public_key (v))
-      end
+let get_manager_key c contract =
+  Storage.Contract.Manager.get_option c contract >>=? function
+  | None -> failwith "get_manager_key"
+  | Some (Manager_repr.Hash _) -> fail (Unrevealed_manager_key contract)
+  | Some (Manager_repr.Public_key v) -> return v
+
+let reveal_manager_key c contract public_key =
+  Storage.Contract.Manager.get c contract >>=? function
+  | Public_key _ -> fail (Previously_revealed_key contract)
+  | Hash v ->
+      let actual_hash = Signature.Public_key.hash public_key in
+      if (Signature.Public_key_hash.equal actual_hash v) then
+        let v = (Manager_repr.Public_key public_key) in
+        Storage.Contract.Manager.set c contract v >>=? fun c ->
+        return c
+      else fail (Inconsistent_hash (public_key,v,actual_hash))
 
 let get_balance c contract =
   Storage.Contract.Balance.get_option c contract >>=? function

src/proto_alpha/lib_protocol/src/contract_storage.mli

@@ -13,14 +13,21 @@ type error +=
   | Counter_in_the_future of Contract_repr.contract * int32 * int32 (* `Temporary *)
   | Unspendable_contract of Contract_repr.contract (* `Permanent *)
   | Non_existing_contract of Contract_repr.contract (* `Temporary *)
+  | Empty_implicit_contract of Signature.Public_key_hash.t (* `Temporary *)
   | Inconsistent_hash of Signature.Public_key.t * Signature.Public_key_hash.t * Signature.Public_key_hash.t (* `Permanent *)
   | Inconsistent_public_key of Signature.Public_key.t * Signature.Public_key.t (* `Permanent *)
   | Missing_public_key of Signature.Public_key_hash.t (* `Permanent *)
   | Failure of string (* `Permanent *)
+  | Previously_revealed_key of Contract_repr.t (* `Permanent *)
+  | Unrevealed_manager_key of Contract_repr.t (* `Permanent *)
 
 val exists: Raw_context.t -> Contract_repr.t -> bool tzresult Lwt.t
 val must_exist: Raw_context.t -> Contract_repr.t -> unit tzresult Lwt.t
 
+val allocated: Raw_context.t -> Contract_repr.t -> bool tzresult Lwt.t
+val must_be_allocated: Raw_context.t -> Contract_repr.t -> unit tzresult Lwt.t
+
+
 val list: Raw_context.t -> Contract_repr.t list Lwt.t
 
 val check_counter_increment:
@@ -37,9 +44,12 @@ val is_spendable: Raw_context.t -> Contract_repr.t -> bool tzresult Lwt.t
 val get_manager:
   Raw_context.t -> Contract_repr.t -> Signature.Public_key_hash.t tzresult Lwt.t
 
-val update_manager_key:
-  Raw_context.t -> Contract_repr.t -> Signature.Public_key.t option ->
-  (Raw_context.t * Signature.Public_key.t) tzresult Lwt.t
+val get_manager_key:
+  Raw_context.t -> Contract_repr.t -> Signature.Public_key.t tzresult Lwt.t
+
+val reveal_manager_key:
+  Raw_context.t -> Contract_repr.t -> Signature.Public_key.t  ->
+  Raw_context.t tzresult Lwt.t
 
 val get_balance: Raw_context.t -> Contract_repr.t -> Tez_repr.t tzresult Lwt.t
 val get_counter: Raw_context.t -> Contract_repr.t -> int32 tzresult Lwt.t