From 365172ddb3b9cb4d9d5020041d16329c37f6cfba Mon Sep 17 00:00:00 2001 From: Simon Stone Date: Wed, 24 Feb 2021 20:07:06 +0000 Subject: [PATCH] Stop ERC-20 transfer bug when to/from accounts are the same (#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 --- token-erc-20/chaincode-go/chaincode/token_contract.go | 4 ++++ token-erc-20/chaincode-javascript/lib/tokenERC20.js | 4 ++++ token-erc-20/chaincode-javascript/test/tokenERC20.test.js | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/token-erc-20/chaincode-go/chaincode/token_contract.go b/token-erc-20/chaincode-go/chaincode/token_contract.go index e17f65fa..e746ca8f 100644 --- 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 index 7ca67749..e6840826 100644 --- 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 index 0f02b774..2f2c1e6d 100644 --- 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'));