From 964bc4044a58faf9fecc041d1bdd5e07de3a6c4f Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Mon, 3 Sep 2018 07:47:22 -0600 Subject: [PATCH] Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC --- CODE_STYLE.md | 6 ++ contracts/mocks/ERC721ReceiverMock.sol | 10 ++-- contracts/token/ERC721/IERC721Basic.sol | 18 +++--- test/token/ERC721/ERC721Basic.behavior.js | 58 ++++++++++---------- test/token/ERC721/ERC721MintBurn.behavior.js | 12 ++-- 5 files changed, 55 insertions(+), 49 deletions(-) diff --git a/CODE_STYLE.md b/CODE_STYLE.md index 593f35cb2..3eca35f86 100644 --- a/CODE_STYLE.md +++ b/CODE_STYLE.md @@ -24,6 +24,12 @@ Any exception or additions specific to our project are documented below. } ``` + The exception are the parameters of events. There is no chance of ambiguity + with these, so they should not have underscores. Not even if they are + specified on an ERC with underscores; removing them doesn't change the ABI, + so we should be consistent with the rest of the events in this repository + and remove them. + * Internal and private state variables should have an underscore suffix. ``` diff --git a/contracts/mocks/ERC721ReceiverMock.sol b/contracts/mocks/ERC721ReceiverMock.sol index a29d0ca2f..3d747f32b 100644 --- a/contracts/mocks/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721ReceiverMock.sol @@ -8,11 +8,11 @@ contract ERC721ReceiverMock is IERC721Receiver { bool internal reverts_; event Received( - address _operator, - address _from, - uint256 _tokenId, - bytes _data, - uint256 _gas + address operator, + address from, + uint256 tokenId, + bytes data, + uint256 gas ); constructor(bytes4 _retval, bool _reverts) public { diff --git a/contracts/token/ERC721/IERC721Basic.sol b/contracts/token/ERC721/IERC721Basic.sol index 6b53f1075..50a5c7f53 100644 --- a/contracts/token/ERC721/IERC721Basic.sol +++ b/contracts/token/ERC721/IERC721Basic.sol @@ -40,19 +40,19 @@ contract IERC721Basic is IERC165 { */ event Transfer( - address indexed _from, - address indexed _to, - uint256 indexed _tokenId + address indexed from, + address indexed to, + uint256 indexed tokenId ); event Approval( - address indexed _owner, - address indexed _approved, - uint256 indexed _tokenId + address indexed owner, + address indexed approved, + uint256 indexed tokenId ); event ApprovalForAll( - address indexed _owner, - address indexed _operator, - bool _approved + address indexed owner, + address indexed operator, + bool approved ); function balanceOf(address _owner) public view returns (uint256 _balance); diff --git a/test/token/ERC721/ERC721Basic.behavior.js b/test/token/ERC721/ERC721Basic.behavior.js index 969ddceed..765614afc 100644 --- a/test/token/ERC721/ERC721Basic.behavior.js +++ b/test/token/ERC721/ERC721Basic.behavior.js @@ -92,17 +92,17 @@ function shouldBehaveLikeERC721Basic (accounts) { it('emit only a transfer event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Transfer'); - logs[0].args._from.should.be.equal(owner); - logs[0].args._to.should.be.equal(this.to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.from.should.be.equal(owner); + logs[0].args.to.should.be.equal(this.to); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); } else { it('emits only a transfer event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Transfer'); - logs[0].args._from.should.be.equal(owner); - logs[0].args._to.should.be.equal(this.to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.from.should.be.equal(owner); + logs[0].args.to.should.be.equal(this.to); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); } @@ -167,9 +167,9 @@ function shouldBehaveLikeERC721Basic (accounts) { it('emits only a transfer event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Transfer'); - logs[0].args._from.should.be.equal(owner); - logs[0].args._to.should.be.equal(owner); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.from.should.be.equal(owner); + logs[0].args.to.should.be.equal(owner); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); it('keeps the owner balance', async function () { @@ -247,10 +247,10 @@ function shouldBehaveLikeERC721Basic (accounts) { result.receipt.logs.length.should.be.equal(2); const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); log.event.should.be.equal('Received'); - log.args._operator.should.be.equal(owner); - log.args._from.should.be.equal(owner); - log.args._tokenId.toNumber().should.be.equal(tokenId); - log.args._data.should.be.equal(data); + log.args.operator.should.be.equal(owner); + log.args.from.should.be.equal(owner); + log.args.tokenId.toNumber().should.be.equal(tokenId); + log.args.data.should.be.equal(data); }); it('should call onERC721Received from approved', async function () { @@ -258,10 +258,10 @@ function shouldBehaveLikeERC721Basic (accounts) { result.receipt.logs.length.should.be.equal(2); const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); log.event.should.be.equal('Received'); - log.args._operator.should.be.equal(approved); - log.args._from.should.be.equal(owner); - log.args._tokenId.toNumber().should.be.equal(tokenId); - log.args._data.should.be.equal(data); + log.args.operator.should.be.equal(approved); + log.args.from.should.be.equal(owner); + log.args.tokenId.toNumber().should.be.equal(tokenId); + log.args.data.should.be.equal(data); }); describe('with an invalid token id', function () { @@ -334,9 +334,9 @@ function shouldBehaveLikeERC721Basic (accounts) { it('emits an approval event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Approval'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._approved.should.be.equal(address); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.owner.should.be.equal(sender); + logs[0].args.approved.should.be.equal(address); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); }; @@ -447,9 +447,9 @@ function shouldBehaveLikeERC721Basic (accounts) { logs.length.should.be.equal(1); logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._operator.should.be.equal(operator); - logs[0].args._approved.should.equal(true); + logs[0].args.owner.should.be.equal(sender); + logs[0].args.operator.should.be.equal(operator); + logs[0].args.approved.should.equal(true); }); }); @@ -469,9 +469,9 @@ function shouldBehaveLikeERC721Basic (accounts) { logs.length.should.be.equal(1); logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._operator.should.be.equal(operator); - logs[0].args._approved.should.equal(true); + logs[0].args.owner.should.be.equal(sender); + logs[0].args.operator.should.be.equal(operator); + logs[0].args.approved.should.equal(true); }); it('can unset the operator approval', async function () { @@ -497,9 +497,9 @@ function shouldBehaveLikeERC721Basic (accounts) { logs.length.should.be.equal(1); logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args._owner.should.be.equal(sender); - logs[0].args._operator.should.be.equal(operator); - logs[0].args._approved.should.equal(true); + logs[0].args.owner.should.be.equal(sender); + logs[0].args.operator.should.be.equal(operator); + logs[0].args.approved.should.equal(true); }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index faf9d756f..c165f1703 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -40,9 +40,9 @@ function shouldBehaveLikeMintAndBurnERC721 (accounts) { it('emits a transfer event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Transfer'); - logs[0].args._from.should.be.equal(ZERO_ADDRESS); - logs[0].args._to.should.be.equal(to); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.from.should.be.equal(ZERO_ADDRESS); + logs[0].args.to.should.be.equal(to); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); }); @@ -78,9 +78,9 @@ function shouldBehaveLikeMintAndBurnERC721 (accounts) { it('emits a burn event', async function () { logs.length.should.be.equal(1); logs[0].event.should.be.equal('Transfer'); - logs[0].args._from.should.be.equal(sender); - logs[0].args._to.should.be.equal(ZERO_ADDRESS); - logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args.from.should.be.equal(sender); + logs[0].args.to.should.be.equal(ZERO_ADDRESS); + logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); });