solidblu1992 / ethereum

GNU General Public License v3.0
18 stars 3 forks source link

Errors trying to transfer value with CT #4

Closed darioAnongba closed 6 years ago

darioAnongba commented 6 years ago

Hi,

I'm Dario, we've exchanged messages via reddit some time ago about confidential transactions. I've now started the implementation and I'm sadly stuck with a problem I don't know how to solve. I've used your solidity contracts and Python scripts to achieve this (they are great!). I was wondering if you could help me on this.

Basically, my implementation is less complex than yours as I can reuse addresses and do not need to use stealth addresses or create a new address for every transaction.

I am currently trying to mint some tokens to some address, create Pedersen commitments (with ct.py), prove that those PCs are positive and transfer the tokens to some other address.

Everything seems to work fine until the transfer() function returns false on this line:

if ((sumInputs[0] != sumOutputs[0]) || (sumInputs[1] != sumOutputs[1])) return false;

I mint 100 tokens (with 2 decimals so 10000) and create Pedersen commitments for values 80 and 20 (with pow10 = 2). The initial blinding factor while minting is 0.

Here is my CTToken.sol contract implementation:

pragma solidity ^0.4.22;

import "openzeppelin-solidity/contracts/lifecycle/Destructible.sol";
import "openzeppelin-solidity/contracts/math/SafeMath.sol";
import "./ECMathInterface.sol";
import "./MLSAGVerifyInterface.sol";

contract CTToken is ECMathInterface, MLSAGVerifyInterface, Destructible {
  using SafeMath for uint;

  constructor(address _ecMathAddr, address _mlsagAddr) ECMathInterface(_ecMathAddr) MLSAGVerifyInterface(_mlsagAddr) public {}

  /* --- Constants --- */
  string public constant name = "Swiss Franc Token";
  string public constant symbol = "CHFT";
  uint8 public constant decimals = 2;

  /* --- Structures and variables --- */
  uint256 public totalSupply;
  uint256 private maxWithdrawUser = 500 * (10 ** uint(decimals));
  mapping (address => uint) private allowedToBurn;

  // Balances in the form of Pedersen commitments
    mapping (address => uint) public committedBalances;

    // Map of commitments which have been proven to be positive
    mapping (uint256 => bool) public positiveBalances;

  /* --- Events --- */
  event Mint(address indexed to, uint256 value);
  event Burn(uint256 value);
  event Withdraw(address indexed from, uint256 value, bool isProcessed);
  event PCRangeProven(uint256 _commitment, uint256 _min, uint256 _max, uint256 _resolution);
  event Transfer(address indexed from, address indexed to, uint256 value);
  event MaxWithdrawUserChanged(uint256 value);

  /* --- Modifiers --- */

  /* --- Public functions --- */

  /**
  * @dev mint tokens to the specified address account. The deposited amount is NOT confidential, the initial blinding factor is 0.
  * @param _to The address that will receive the minted tokens.
  * @param _value The amount of tokens to mint.
  * @return A boolean that indicates if the operation was successful.
  */
  function mint(address _to, uint256 _value) onlyOwner public returns (bool) {
    // The central bank cannot mint to itself
    if (_to == owner) return false;

    updateBalanceCheckEmpty(_to, ecMath.MultiplyH(_value));
    totalSupply = totalSupply.add(_value);
    emit Mint(_to, _value);

    return true;
  }

  /**
  * @dev mint tokens to the specified addresses accounts. Allows multiple deposits at once.
  * The deposited amount is NOT confidential, the initial blinding factor is 0.
  *
  * @param _beneficiaries The addresses that will receive the minted tokens.
  * @param _values The amount of tokens to mint for each address.
  * @return A boolean that indicates if the operation was successful.
  */
  function mintMultiple(address[] _beneficiaries, uint256[] _values) onlyOwner public returns (bool) {  
    // One value per address
    require(_beneficiaries.length == _values.length);

    for (uint256 i = 0; i < _beneficiaries.length; i++) {
      mint(_beneficiaries[i], _values[i]);
    }

    return true;
  }

  /**
  * @dev Set max amount of tokens a user can redeem at once.
  * @param _amount uint256 amount of tokens.
  */
  function setMaxWithdrawUser(uint256 _amount) onlyOwner public returns (bool) {
    require (_amount > 0);

    maxWithdrawUser = _amount;
    emit MaxWithdrawUserChanged(_amount);
    return true;
  }

  /**
  * @dev transfer token for a specified address. Both PCs must have been proved to be positive.
  * @param _to The address to transfer to.
  * @param _pcTo Pedersen commitment for amount to transfer.
  * @param _pcRemaining Pedersen commitment for amount remaining.
  */
  function transfer(address _to, uint256 _pcTo, uint256 _pcRemaining) public returns (bool) {
    // Address cannot be 0, same for commitments
    require(_to != address(0) && _pcTo != 0 && _pcRemaining != 0);

    // Check that new PCs are positive
      if (!positiveBalances[_pcTo] || !positiveBalances[_pcRemaining]) return false;

    // Sum of input commitments (current balance) must be equal to the sum of output commitments (pcTo + pcRemaining)
    uint256[2] memory sumInputs = ecMath.ExpandPoint(committedBalances[msg.sender]);
    uint256[2] memory pcToExpanded = ecMath.ExpandPoint(_pcTo);
    uint256[2] memory sumOutputs = ecMath.Add(pcToExpanded, ecMath.ExpandPoint(_pcRemaining));
    if ((sumInputs[0] != sumOutputs[0]) || (sumInputs[1] != sumOutputs[1])) return false;

    // Update balances
    committedBalances[msg.sender] = _pcRemaining;
    updateBalanceCheckEmpty(_to, pcToExpanded);

    // TODO: Add the encrypted data to the event and as argument to the function
    emit Transfer(msg.sender, _to, _pcTo);

    return true;
  }

  /**
  * @dev transfer all tokens of to a specified address.
  * @param _to The address to transfer to.
  */
  function transferAll(address _to) public returns (bool) {
    require(_to != address(0));

    // Update sending token balance
      uint256[2] memory pcValue = ecMath.ExpandPoint(committedBalances[msg.sender]);
      committedBalances[msg.sender] = 0;
    updateBalanceCheckEmpty(_to, pcValue);

    return true;
  }

  /**
  * @dev Gets the balance of the specified address.
  * @param _owner The address to query the the balance of.
  * @return An uint256 representing the pedersen commitment of the amont owned by the passed address.
  */
  function balanceOf(address _owner) public view returns (uint256) {
    return committedBalances[_owner];
  }

  function PCProvePositive(uint256[2] total_commit, uint256 power10, uint256 offset, uint256[] bit_commits, uint256[] signature) public returns (bool success) {
    //Get number of bits to prove
    if(bit_commits.length % 2 != 0) return false;
    uint256 bits = (bit_commits.length / 2);
    if (bits == 0) return false;

    //Impose limits on inputs in order to avoid values greater than Ncurve // 2
    if (power10 > 35) return false;
    if (offset > (ecMath.GetNCurve() / 4)) return false;
    if (bits > 64) return false;

    //Check for proper signature size
    if (signature.length != (4*bits+1)) return false;

    //Check that bitwise commitments add up to total commitment
    uint256 i;
    uint256[2] memory temp1;
    (temp1[0], temp1[1]) = (bit_commits[0], bit_commits[1]);
    for (i = 1; i < bits; i++) {
      temp1 = ecMath.Add(temp1, [bit_commits[2*i], bit_commits[2*i+1]]);
    }

    if (offset > 0) {
      temp1 = ecMath.AddMultiplyH(temp1, offset);
    }

    if ( (total_commit[0] != temp1[0]) || (total_commit[1] != temp1[1]) ) return false;

    //Build Public Keys for Signature Verification
    uint256[] memory P = new uint256[](8*bits);
    uint256[2] memory temp2;
    for (i = 0; i < bits; i++) {
        //Store bitwise commitment
        temp1 = [bit_commits[2*i], bit_commits[2*i+1]];
        (P[2*i], P[2*i+1]) = (temp1[0], temp1[1]);

        //Calculate -(4**bit)*(10**power10)*H
        temp2 = ecMath.MultiplyH((4**i)*(10**power10));
        temp2 = ecMath.Negate(temp2);

        //Calculate 1st counter commitment: C' = C - (4**bit)*(10**power10)*H
        temp1 = ecMath.Add(temp1, temp2);
        (P[2*(i+bits)], P[2*(i+bits)+1]) = (temp1[0], temp1[1]);

        //Calculate 2nd counter commitment: C'' = C - 2*(4**bit)*(10**power10)*H
        temp1 = ecMath.Add(temp1, temp2);
        (P[2*(i+2*bits)], P[2*(i+2*bits)+1]) = (temp1[0], temp1[1]);

        //Calculate 3rd counter commitment: C''' = C - 3*(4**bit)*(10**power10)*H
        temp1 = ecMath.Add(temp1, temp2);
        (P[2*(i+3*bits)], P[2*(i+3*bits)+1]) = (temp1[0], temp1[1]);
    }

    //Verify Signature
    total_commit[0] = ecMath.CompressPoint(total_commit);
    success = mlsagVerify.VerifyMSAG(bits, bytes32(ecMath.CompressPoint(total_commit)), P, signature);

    if (success) {
        positiveBalances[total_commit[0]] = true;
        temp1[0] = (4**bits-1)*(10**power10)+offset;    //Max value
        temp1[1] = (10**power10);                       //Resolution
        emit PCRangeProven(total_commit[0], offset, temp1[0], temp1[1]);
    }
  }

  /**
  * @dev update the balance of recipient checking if it is not empty. If not, adds PC to current balance.
  * @param _to The destination address.
  * @param _pcValue The pedersen commitment to update the destination account.
  */
  function updateBalanceCheckEmpty(address _to, uint256[2] memory _pcValue) internal returns (bool) {
    if (committedBalances[_to] != 0) {
      uint256[2] memory pcCurrentBalance = ecMath.ExpandPoint(committedBalances[_to]);
      _pcValue = ecMath.Add(_pcValue, pcCurrentBalance);
    }
    committedBalances[_to] = ecMath.CompressPoint(_pcValue);

    return true;
  }
}

Of course it's no problem if you do not have time to review this. I know I am missing something obvious...

Cheers, Dario

solidblu1992 commented 6 years ago

Hi Dario,

What blinding factors are you using for the _pcTo and _pcRemaining? In my RingCT implementation, the blinding factors generated by my scripts do not add up to the sum of the input blinding factors. This is because if they did then RingCT would lose anonymity. What you are doing in your case is fine for just straight CT, but my guess is that you will need to modify my scripts for your use case.

What I have: 10000 H = (8000 H + BF1 G1) + (2000 H + BF2 * G1); where BF1 + BF2 != 0 mod Ncurve

What you need: 10000 H = (8000 H + BF G1) + (2000 H + [Ncurve-BF] * G1);

darioAnongba commented 6 years ago

Hi,

Thanks for the quick answer! Indeed I just realized my mistake by reading through this: https://www.elementsproject.org/elements/confidential-transactions/investigation.html

C(BF1, data1) + C(BF2, data2) == C(BF1 + BF2, data1 + data2)
C(BF1, data1) - C(BF1, data1) == 0

Thank you again!

darioAnongba commented 6 years ago

Also, I just realized that if I reuse the same addresses instead of sending amounts to empty addresses, I could run into a problem where an account that constantly receives payments is unable to create a new transaction because between the moment he creates the transaction and the moment it is broadcast on the network, he would need a new blinding factor !

What do you think of this problem?

solidblu1992 commented 6 years ago

Yes, I noticed that during development too. This is one reason CT has good synergy with Stealth Addresses. If every transaction has to be to a new and empty address they you don't run into this problem.

Perhaps another solution is to change the committedBalances mapping to (address => uint[]) and keep track of each UTXO separately. For example, each incoming transaction creates a new element in the uint[] array and each outgoing transaction removes certain elements from the array.

darioAnongba commented 6 years ago

The problem with generating a new address for each transaction is that it is not possible to use addresses (standard Ethereum addresses) as reference (some sort of identity).

Yes the second solution is definitely what I will end up doing. It's crazy how the more we advance the more we actually need to recreate the UTXO system of Bitcoin. Satoshi had everything right.