Stop ERC-20 transfer bug when to/from accounts are the same (hyperled…

…ger#425) Using the ERC-20 sample, you can submit a transfer to and from the same account. Because the code doesn't handle this, it ends up minting new tokens into that account. The correct behaviour is not specified by the ERC-20 specification, although the OpenZeppelin implementation seems to permit it. IMO we should just block it with an error because I can't see a use case for allowing it and it is most likely a user error. Signed-off-by: Simon Stone <[email protected]>
manish-sethi · Feb 24, 2021 · 365172d · 365172d
1 parent 651f223
commit 365172d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 0 deletions.
diff --git a/token-erc-20/chaincode-go/chaincode/token_contract.go b/token-erc-20/chaincode-go/chaincode/token_contract.go
@@ -427,6 +427,10 @@ func (s *SmartContract) TransferFrom(ctx contractapi.TransactionContextInterface
 // Dependant functions include Transfer and TransferFrom
 func transferHelper(ctx contractapi.TransactionContextInterface, from string, to string, value int) error {
 
+	if from == to {
+		return fmt.Errorf("cannot transfer to and from same client account")
+	}
+
 	if value < 0 { // transfer of 0 is allowed in ERC-20, so just validate against negative amounts
 		return fmt.Errorf("transfer amount cannot be negative")
 	}

diff --git a/token-erc-20/chaincode-javascript/lib/tokenERC20.js b/token-erc-20/chaincode-javascript/lib/tokenERC20.js
@@ -161,6 +161,10 @@ class TokenERC20Contract extends Contract {
 
     async _transfer(ctx, from, to, value) {
 
+        if (from === to) {
+            throw new Error('cannot transfer to and from same client account');
+        }
+
         // Convert value from string to int
         const valueInt = parseInt(value);
 

diff --git a/token-erc-20/chaincode-javascript/test/tokenERC20.test.js b/token-erc-20/chaincode-javascript/test/tokenERC20.test.js
@@ -94,6 +94,12 @@ describe('Chaincode', () => {
     });
 
     describe('#_transfer', () => {
+
+        it('should fail when the sender and the receipient are the same', async () => {
+            await expect(token._transfer(ctx, 'Alice', 'Alice', '1000'))
+                .to.be.rejectedWith(Error, 'cannot transfer to and from same client account');
+        });
+
         it('should fail when the sender does not have enough token', async () => {
             mockStub.createCompositeKey.withArgs('balance', ['Alice']).returns('balance_Alice');
             mockStub.getState.withArgs('balance_Alice').resolves(Buffer.from('500'));