flamingo-finance / flamingo-contract-swap

7 stars 5 forks source link

Issue 5 #5

Open Edward-Lo opened 4 years ago

Edward-Lo commented 4 years ago

Description

The FlamingoSwapPairContract.Nep5 contract implements the Transfer() function following the NEP-5 standard.

However, the current implementation has two minor issues which violate the NEP-5 standard.

private static bool Transfer(byte[] from, byte[] to, BigInteger amount, byte[] callscript)
{
    //Check parameters
    Assert(from.Length == 20 && to.Length == 20, "The parameters from and to SHOULD be 20-byte addresses.");
    Assert(amount > 0, "The parameter amount MUST be greater than 0.");

    if (!Runtime.CheckWitness(from) && from.AsBigInteger() != callscript.AsBigInteger())
        return false;
    StorageMap asset = Storage.CurrentContext.CreateMap(BalanceMapKey);
    var fromAmount = asset.Get(from).AsBigInteger();
    if (fromAmount < amount)
        return false;
    if (from == to)
        return true;
    ...
}
  1. Transfer() should allow zero amount transfers with corresponding event emitted, but the assertion Assert(amount > 0, "...") forbids this, which violates the NEP-5.
  2. When the from and to addresses are the same, current implementation fails to fire the event but simply return true.

Recommendation

Fix the violations

private static bool Transfer(byte[] from, byte[] to, BigInteger amount, byte[] callscript)
{
    //Check parameters
    Assert(from.Length == 20 && to.Length == 20, "The parameters from and to SHOULD be 20-byte addresses.");
    Assert(amount >= 0, "The parameter amount MUST be greater than 0.");

    if (!Runtime.CheckWitness(from) && from.AsBigInteger() != callscript.AsBigInteger())
        return false;
    StorageMap asset = Storage.CurrentContext.CreateMap(BalanceMapKey);
    var fromAmount = asset.Get(from).AsBigInteger();
    if (fromAmount < amount)
        return false;
    if (from == to) {
        Transferred(from, to, amount);
        return true;
    }
    ...
}
Ashuaidehao commented 4 years ago

Update Assert(amount >= 0, "The parameter amount MUST be greater than 0."); .