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.

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

[Release] Fix overflows in the transaction snark #6034

Merged
merged 2 commits into from
Sep 18, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions src/lib/transaction_snark/transaction_snark.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,7 @@ module Base = struct
in
Amount.Checked.sub base_amount coinbase_receiver_fee)
in
let receiver_overflow = ref Boolean.false_ in
let%bind root_after_receiver_update =
[%with_label "Update receiver"]
(Frozen_ledger_hash.modify_account_recv
Expand Down Expand Up @@ -1538,11 +1539,34 @@ module Base = struct
~then_:Amount.(var_of_t zero)
~else_:amount_for_new_account
in
(* TODO: This case can be contrived using minted tokens, handle
it in the transaction logic and add a case for it to
[User_command_failure.t].
*)
Balance.Checked.(account.balance + receiver_amount)
(* NOTE: Instead of capturing this as part of the user command
failures, we capture it inline here and bubble it out to a
reference. This behavior is still in line with the
out-of-snark transaction logic.

Updating [user_command_fails] to include this value from here
onwards will ensure that we do not update the source or
receiver accounts. The only places where [user_command_fails]
may have already affected behaviour are
* when the fee-payer is paying the account creation fee, and
* when a new token is created.
In both of these, this account is new, and will have a
balance of 0, so we can guarantee that there is no overflow.
*)
let%bind balance, `Overflow overflow =
Balance.Checked.add_amount_flagged account.balance
receiver_amount
in
let%bind () =
[%with_label "Overflow error only occurs in user commands"]
Boolean.(Assert.any [is_user_command; not overflow])
in
receiver_overflow := overflow ;
Balance.Checked.if_ overflow ~then_:account.balance
~else_:balance
in
let%bind user_command_fails =
Boolean.(!receiver_overflow || user_command_fails)
in
let%bind is_empty_and_writeable =
(* Do not create a new account if the user command will fail. *)
Expand Down Expand Up @@ -1581,6 +1605,9 @@ module Base = struct
; voting_for= account.voting_for
; timing= account.timing } ))
in
let%bind user_command_fails =
Boolean.(!receiver_overflow || user_command_fails)
in
let%bind fee_payer_is_source = Account_id.Checked.equal fee_payer source in
let%bind root_after_source_update =
[%with_label "Update source"]
Expand Down Expand Up @@ -1736,11 +1763,19 @@ module Base = struct
(let user_command_excess =
Signed.Checked.of_unsigned (Checked.of_fee payload.common.fee)
in
let%bind fee_transfer_excess =
let%map magnitude =
Checked.(payload.body.amount + of_fee payload.common.fee)
let%bind fee_transfer_excess, fee_transfer_excess_overflowed =
let%map magnitude, `Overflow overflowed =
Checked.(
add_flagged payload.body.amount (of_fee payload.common.fee))
in
Signed.create ~magnitude ~sgn:Sgn.Checked.neg
(Signed.create ~magnitude ~sgn:Sgn.Checked.neg, overflowed)
in
let%bind () =
(* TODO: Reject this in txn pool before fees-in-tokens. *)
[%with_label "Fee excess does not overflow"]
Boolean.(
Assert.any
[not is_fee_transfer; not fee_transfer_excess_overflowed])
in
Signed.Checked.if_ is_fee_transfer ~then_:fee_transfer_excess
~else_:user_command_excess)
Expand Down