From 65a1a46453eb0155e5bf8b0f58036e7603e9a8d2 Mon Sep 17 00:00:00 2001 From: Daniel Fernandez Date: Thu, 16 May 2019 12:51:56 -0300 Subject: [PATCH 1/2] Contracts constants now use UPPER_CASE_WITH_UNDERSCORES style --- contracts/Accounts.sol | 10 +++++----- contracts/Data.sol | 6 +++--- contracts/Payments.sol | 4 ++-- lib/bat.js | 6 +++--- src/benchmarks.js | 2 +- src/demo.js | 4 ++-- test/test_accounts.js | 32 ++++++++++++++++---------------- test/test_batpay.js | 4 ++-- test/test_payments.js | 22 +++++++++++----------- 9 files changed, 45 insertions(+), 45 deletions(-) diff --git a/contracts/Accounts.sol b/contracts/Accounts.sol index 994f3ea..1250de9 100644 --- a/contracts/Accounts.sol +++ b/contracts/Accounts.sol @@ -62,7 +62,7 @@ contract Accounts is Data { function bulkRegister(uint256 bulkSize, bytes32 rootHash) public { require(bulkSize > 0, "Bulk size can't be zero"); require(bulkSize < params.maxBulk, "Cannot register this number of ids simultaneously"); - require(SafeMath.add(accounts.length, bulkSize) <= maxAccountId, "Cannot register: ran out of ids"); + require(SafeMath.add(accounts.length, bulkSize) <= MAX_ACCOUNT_ID, "Cannot register: ran out of ids"); require(rootHash > 0, "Root hash can't be zero"); emit BulkRegister(bulkSize, accounts.length, bulkRegistrations.length); @@ -97,7 +97,7 @@ contract Accounts is Data { * @return the id of the new account */ function register() public returns (uint32 ret) { - require(accounts.length < maxAccountId, "no more accounts left"); + require(accounts.length < MAX_ACCOUNT_ID, "no more accounts left"); ret = (uint32)(accounts.length); accounts.push(Account(msg.sender, 0, 0)); emit AccountRegistered(ret, msg.sender); @@ -128,16 +128,16 @@ contract Accounts is Data { * @dev Deposit tokens into the BatchPayment contract to enable scalable payments * @param amount Amount of tokens to deposit on `accountId`. User should have * enough balance and issue an `approve()` method prior to calling this. - * @param accountId The id of the user account. In case `newAccountFlag` is used, + * @param accountId The id of the user account. In case `NEW_ACCOUNT_FLAG` is used, * a new account will be registered and the requested amount will be * deposited in a single operation. */ function deposit(uint64 amount, uint256 accountId) external { - require(accountId < accounts.length || accountId == newAccountFlag, "invalid accountId"); + require(accountId < accounts.length || accountId == NEW_ACCOUNT_FLAG, "invalid accountId"); require(amount > 0, "amount should be positive"); require(token.transferFrom(msg.sender, address(this), amount), "transfer failed"); - if (accountId == newAccountFlag) { + if (accountId == NEW_ACCOUNT_FLAG) { // new account uint newId = register(); accounts[newId].balance = amount; diff --git a/contracts/Data.sol b/contracts/Data.sol index a28039c..fd38e0b 100644 --- a/contracts/Data.sol +++ b/contracts/Data.sol @@ -69,8 +69,8 @@ contract Data { Config public params; address public owner; - uint public constant maxAccountId = 2**32-1; // Maximum account id (32-bits) - uint public constant newAccountFlag = 2**256-1; // Request registration of new account - uint public constant instantSlot = 32768; + uint public constant MAX_ACCOUNT_ID = 2**32-1; // Maximum account id (32-bits) + uint public constant NEW_ACCOUNT_FLAG = 2**256-1; // Request registration of new account + uint public constant INSTANT_SLOT = 32768; } diff --git a/contracts/Payments.sol b/contracts/Payments.sol index 2c6b9c7..7341eae 100644 --- a/contracts/Payments.sol +++ b/contracts/Payments.sol @@ -236,7 +236,7 @@ contract Payments is Accounts { uint64 needed = params.collectStake; // check if this is an instant collect - if (slotId >= instantSlot) { + if (slotId >= INSTANT_SLOT) { uint64 declaredAmountLessFee = SafeMath.sub64(declaredAmount, fee); sl.delegateAmount = declaredAmount; needed = SafeMath.add64(needed, declaredAmountLessFee); @@ -260,7 +260,7 @@ contract Payments is Accounts { balanceSub(delegate, needed); // Proceed if the user is withdrawing its balance - if (destination != address(0) && slotId >= instantSlot) { + if (destination != address(0) && slotId >= INSTANT_SLOT) { uint64 toWithdraw = accounts[toAccountId].balance; accounts[toAccountId].balance = 0; require(token.transfer(destination, toWithdraw), "transfer failed"); diff --git a/lib/bat.js b/lib/bat.js index 6a4cf97..bf467c1 100644 --- a/lib/bat.js +++ b/lib/bat.js @@ -4,7 +4,7 @@ var abi = require('ethereumjs-abi') const bytesPerId = 4 -var newAccountFlag = new BigNumber(2).pow(256).minus(1) +var NEW_ACCOUNT_FLAG = new BigNumber(2).pow(256).minus(1) const prefs = { default: { @@ -147,7 +147,7 @@ class BP { this.unlockBlocks = params[6].toNumber() this.maxCollectAmount = params[13].toNumber() - this.instantSlot = (await this.bp.instantSlot.call()).toNumber() + this.INSTANT_SLOT = (await this.bp.INSTANT_SLOT.call()).toNumber() } async register (addr) { @@ -389,7 +389,7 @@ class BP { } module.exports = { - newAccountFlag, + NEW_ACCOUNT_FLAG, BP, findEvent, getChallengeData, diff --git a/src/benchmarks.js b/src/benchmarks.js index aa81e07..e8460a1 100644 --- a/src/benchmarks.js +++ b/src/benchmarks.js @@ -119,7 +119,7 @@ async function collect () { let t3 = await b.collect(id, 2, id2, 2, 3, 100, 2, acc[0]) addStat('collect-empty-withdraw', t3.receipt.gasUsed) - let t7 = await b.collect(id, b.instantSlot, id2, 3, 4, 100, 2, acc[0]) + let t7 = await b.collect(id, b.INSTANT_SLOT, id2, 3, 4, 100, 2, acc[0]) addStat('collect-empty-instant-withdraw', t7.receipt.gasUsed) await utils.skipBlocks(b.challengeBlocks) diff --git a/src/demo.js b/src/demo.js index b8dd93b..26006f6 100644 --- a/src/demo.js +++ b/src/demo.js @@ -156,7 +156,7 @@ async function doStuff () { let amount = await b.getCollectAmount(i, c, max) if (i == 3) amount = amount + 100 - await b.collect(0, i + b.instantSlot, i, c, max, amount, 1, addr) + await b.collect(0, i + b.INSTANT_SLOT, i, c, max, amount, 1, addr) } await b.showBalance() @@ -164,7 +164,7 @@ async function doStuff () { await skipBlocks(b.challengeBlocks) console.log('Freeing collect slots') for (let i = 1; i <= 5; i++) { - await b.freeSlot(0, i + b.instantSlot) + await b.freeSlot(0, i + b.INSTANT_SLOT) } await b.showBalance() } catch (e) { diff --git a/test/test_accounts.js b/test/test_accounts.js index d039342..804d103 100644 --- a/test/test_accounts.js +++ b/test/test_accounts.js @@ -18,7 +18,7 @@ contract('Accounts', (addr) => { let a1 = addr[1] let bp, tAddress, st - const newAccountFlag = new BigNumber(2).pow(256).minus(1) + const NEW_ACCOUNT_FLAG = new BigNumber(2).pow(256).minus(1) before(async () => { await utils.skipBlocks(1) @@ -35,10 +35,10 @@ contract('Accounts', (addr) => { it('Should fail on not enough approval', async () => { const amount = 100 await st.approve(bp.address, amount - 1) - await catchRevert(bp.deposit(amount, newAccountFlag)) + await catchRevert(bp.deposit(amount, NEW_ACCOUNT_FLAG)) await st.approve(bp.address, 0) - await catchRevert(bp.deposit(amount, newAccountFlag)) + await catchRevert(bp.deposit(amount, NEW_ACCOUNT_FLAG)) }) it('Should accept deposits for new accounts', async () => { @@ -46,7 +46,7 @@ contract('Accounts', (addr) => { const amount = 100 let r0 = await st.approve(bp.address, amount) - let r1 = await bp.deposit(amount, newAccountFlag) + let r1 = await bp.deposit(amount, NEW_ACCOUNT_FLAG) let v0 = await st.balanceOf.call(a0) let v1 = await st.balanceOf.call(bp.address) @@ -60,7 +60,7 @@ contract('Accounts', (addr) => { const amount = 100 let r0 = await st.approve(bp.address, 2 * amount) - let r1 = await bp.deposit(amount, newAccountFlag) + let r1 = await bp.deposit(amount, NEW_ACCOUNT_FLAG) let v0 = await bp.balanceOf.call(0) await bp.deposit(amount, 0) @@ -71,7 +71,7 @@ contract('Accounts', (addr) => { }) it('Should reject 0-token deposits', async () => { - await assertRequire(bp.deposit(0, newAccountFlag), 'amount should be positive') + await assertRequire(bp.deposit(0, NEW_ACCOUNT_FLAG), 'amount should be positive') }) }) @@ -80,7 +80,7 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - await bp.deposit(amount, newAccountFlag) + await bp.deposit(amount, NEW_ACCOUNT_FLAG) let id = await bp.getAccountsLength.call() id = id.toNumber() - 1 @@ -105,10 +105,10 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - await bp.deposit(amount, newAccountFlag) + await bp.deposit(amount, NEW_ACCOUNT_FLAG) let id = await bp.getAccountsLength.call() - id = id.toNumber() - 1 // this is a dangerous way to obtain the ID of the newAccountFlag, as many accounts c + id = id.toNumber() - 1 // this is a dangerous way to obtain the ID of the NEW_ACCOUNT_FLAG, as many accounts c await bp.withdraw(1, id) // make sure we can actually do a withdraw using a valid id await catchRevert(bp.withdraw(amount / 2, id + 1)) // try with invalid id @@ -118,7 +118,7 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - await bp.deposit(amount, newAccountFlag) + await bp.deposit(amount, NEW_ACCOUNT_FLAG) let id = await bp.getAccountsLength.call() id = id.toNumber() - 1 @@ -133,7 +133,7 @@ contract('Accounts', (addr) => { // const amount = 100; // // await st.approve(bp.address, amount); - // await bp.deposit(amount, newAccountFlag); + // await bp.deposit(amount, NEW_ACCOUNT_FLAG); // // let id = await bp.getAccountsLength.call(); // id = id.toNumber()-1; @@ -155,7 +155,7 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - await bp.deposit(amount, newAccountFlag) + await bp.deposit(amount, NEW_ACCOUNT_FLAG) let id = await bp.getAccountsLength.call() id = id.toNumber() - 1 @@ -175,9 +175,9 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - let tx1 = await bp.deposit(1, newAccountFlag) + let tx1 = await bp.deposit(1, NEW_ACCOUNT_FLAG) const v1 = await bp.getAccountsLength.call() - let tx2 = await bp.deposit(1, newAccountFlag) + let tx2 = await bp.deposit(1, NEW_ACCOUNT_FLAG) const v2 = await bp.getAccountsLength.call() eventEmitted(tx1, 'AccountRegistered') @@ -225,7 +225,7 @@ contract('Accounts', (addr) => { }) // TODO: check case we run out of ids: - // require(accounts.length + n <= maxAccountId, "Cannot register: ran out of ids"); + // require(accounts.length + n <= MAX_ACCOUNT_ID, "Cannot register: ran out of ids"); it('Bulk registration should fail for n == 0', async () => { let v0 = await bp.getBulkLength.call() @@ -261,7 +261,7 @@ contract('Accounts', (addr) => { }) // TODO: check case we registered a lot of accounts - // accounts.length < maxAccountId, "no more accounts left"); + // accounts.length < MAX_ACCOUNT_ID, "no more accounts left"); }) describe('claim', () => { diff --git a/test/test_batpay.js b/test/test_batpay.js index 78b57d5..8fa1644 100644 --- a/test/test_batpay.js +++ b/test/test_batpay.js @@ -50,7 +50,7 @@ contract('BatPay', (addr) => { let a1 = addr[1] let batPay, tAddress, st - const newAccountFlag = new BigNumber(2).pow(256).minus(1) + const NEW_ACCOUNT_FLAG = new BigNumber(2).pow(256).minus(1) before(async () => { let ret = await utils.newInstances() @@ -64,7 +64,7 @@ contract('BatPay', (addr) => { unlockBlocks = params[7].toNumber() challengeBlocks = params[3].toNumber() challengeStepBlocks = params[4].toNumber() - instantSlot = (await batPay.instantSlot.call()).toNumber() + INSTANT_SLOT = (await batPay.INSTANT_SLOT.call()).toNumber() }) it('cannot obtain the balance for invalid id', async () => { diff --git a/test/test_payments.js b/test/test_payments.js index d2cdec5..b919098 100644 --- a/test/test_payments.js +++ b/test/test_payments.js @@ -19,7 +19,7 @@ contract('Payments', (accounts) => { let a1 = accounts[1] let bp, tAddress, st - const newAccountFlag = new BigNumber(2).pow(256).minus(1) + const NEW_ACCOUNT_FLAG = new BigNumber(2).pow(256).minus(1) before(async function () { await utils.skipBlocks(1) @@ -56,7 +56,7 @@ contract('Payments', (accounts) => { // put enough funds to transfer and bulk register ids await st.approve(bp.address, total_amount * 2) - t0 = await bp.deposit(total_amount * 2, newAccountFlag) + t0 = await bp.deposit(total_amount * 2, NEW_ACCOUNT_FLAG) from_id = await bp.getAccountsLength.call() - 1 v0 = await bp.bulkRegister(list.length, rootHash) @@ -181,7 +181,7 @@ contract('Payments', (accounts) => { it('should reject registerPayment if there are too many payees', async () => { // NOTE: there are 2 checks that depend on newCount: // 1. (payData.length-2) / bytesPerId + newCount < maxTransfer = 100000 - // 2. accounts.length + newCount < maxAccountId = 2 ** 32 + // 2. accounts.length + newCount < MAX_ACCOUNT_ID = 2 ** 32 // // because 100000 < 2 ** 32 we can only trigger the first condition let toobig_count = 100000 // this should actually be bp.maxTransfer, however it crashes @@ -292,7 +292,7 @@ contract('Payments', (accounts) => { let mid = userid[1] let amount = await b.getCollectAmount(mid, 0, maxPayIndex + 1) let b0 = (await b.balanceOf(mid)).toNumber() - await b.collect(id, b.instantSlot, mid, 0, maxPayIndex + 1, amount, 0, 0) + await b.collect(id, b.INSTANT_SLOT, mid, 0, maxPayIndex + 1, amount, 0, 0) let b1 = (await b.balanceOf(mid)).toNumber() assert.equal(b0 + amount, b1) @@ -317,7 +317,7 @@ contract('Payments', (accounts) => { it('should pay fee on instant-collect', async () => { let mid = userid[3] - let slot = b.instantSlot + 1 + let slot = b.INSTANT_SLOT + 1 let amount = await b.getCollectAmount(mid, 0, maxPayIndex + 1) let fee = Math.floor(amount / 3) let b0 = (await b.balanceOf(mid)).toNumber() @@ -375,7 +375,7 @@ contract('Payments', (accounts) => { it('should withdraw if requested instant', async () => { let mid = userid[6] - let slot = b.instantSlot + 2 + let slot = b.INSTANT_SLOT + 2 let amount = await b.getCollectAmount(mid, 0, maxPayIndex + 1) let fee = Math.floor(amount / 3) let b0 = (await b.balanceOf(mid)).toNumber() @@ -398,7 +398,7 @@ contract('Payments', (accounts) => { it('should reject if payIndex is less or equal to last collected payment ID', async () => { let collectorAccountId = userid[6] - let slot = b.instantSlot + let slot = b.INSTANT_SLOT let amount = await b.getCollectAmount(collectorAccountId, 0, maxPayIndex + 1) let [, , lastCollectedPaymentId] = await b.getAccount(collectorAccountId) @@ -407,7 +407,7 @@ contract('Payments', (accounts) => { it('should reject if payIndex is invalid', async () => { let collectorId = userid[6] - let slot = b.instantSlot + 2 + let slot = b.INSTANT_SLOT + 2 let amount = await b.getCollectAmount(collectorId, 0, maxPayIndex + 1) let [, , lastCollectedPaymentId] = await b.getAccount(collectorId) const tooHighPayIndex = (await b.getPaymentsLength()) + 1 @@ -428,7 +428,7 @@ contract('Payments', (accounts) => { it('should reject if invalid toAccountId', async () => { let mid = userid[6] - let slot = b.instantSlot + 2 + let slot = b.INSTANT_SLOT + 2 let amount = await b.getCollectAmount(mid, 0, maxPayIndex + 1) const invalidCollectorId = 123454321 // We need a valid address to sign the collect transaction. @@ -442,7 +442,7 @@ contract('Payments', (accounts) => { it('should reject wrong fromPayIndex / Invalid Signature', async () => { let collectorId = userid[7] - let slot = b.instantSlot + 2 + let slot = b.INSTANT_SLOT + 2 let amount = await b.getCollectAmount(collectorId, 0, maxPayIndex + 1) const invalidFromPayIndex = 123454321 @@ -468,7 +468,7 @@ contract('Payments', (accounts) => { utils.skipBlocks(b.unlockBlocks) let b0 = (await b.balanceOf(id)).toNumber() - await b.collect(id, b.instantSlot, id, 0, pid+1, stake, 0, 0) + await b.collect(id, b.INSTANT_SLOT, id, 0, pid+1, stake, 0, 0) let b1 = (await b.balanceOf(id)).toNumber() assert.isBelow(b1, b0) From 36c495adcba935239e11c3457f4ee5d34e660938 Mon Sep 17 00:00:00 2001 From: Daniel Fernandez Date: Thu, 16 May 2019 14:31:32 -0300 Subject: [PATCH 2/2] Constants re-updated after merge --- test/test_accounts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_accounts.js b/test/test_accounts.js index 3ca2af2..b6cf2a6 100644 --- a/test/test_accounts.js +++ b/test/test_accounts.js @@ -105,7 +105,7 @@ contract('Accounts', (addr) => { const amount = 100 await st.approve(bp.address, amount) - await bp.deposit(amount, newAccountFlag) + await bp.deposit(amount, NEW_ACCOUNT_FLAG) let id = await bp.getAccountsLength.call() id = id.toNumber() - 1