MinaFoundation / mina-fungible-token

https://minafoundation.github.io/mina-fungible-token/
Apache License 2.0
18 stars 8 forks source link

transfer() overwrites balanceChange #82

Open kantp opened 3 months ago

kantp commented 3 months ago

The TokenContract.transfer() function produces two account updates: one for the sender of the funds and one for the receiver.

Rather than creating the AccountUpdates from scratch, it can accept a from and to AccountUpdate. In this case, it overwrites the balanceChange field to the amount being transferred.

Snippet from transfer()

async transfer(
  from: PublicKey | AccountUpdate,
  to: PublicKey | AccountUpdate,
  amount: UInt64 | number | bigint
) {
  // [VERIDISE] ... elided

  from.balanceChange = Int64.from(amount).neg();
  to.balanceChange = Int64.from(amount);

  let forest = toForest([from, to]);
  await this.approveBase(forest);
}

However, from and to may have non-zero balanceChanges when passed to transfer(). Setting balanceChange overrides whatever the previous value was.

Impact

If users pass an AccountUpdate with an existing balance change to this function generated by another application, they may experience difficult-to-debug prover errors due to the mutation.

Recommendation

Use AccountUpdate.balance.addInPlace() and AccountUpdate.balance.subInPlace() instead of overwriting balances. Note that, if from and to have existing balances, this will require they are negatives of each other