diff --git a/README.md b/README.md index df55ae4..9ee27e2 100644 --- a/README.md +++ b/README.md @@ -87,10 +87,11 @@ Use mainnet (or goerli) to deploy BillingConnector: ``` hh deploy-billing-connector --network mainnet \ - --tokenGateway \ + --tokengateway \ --billing \ --token \ - --governor + --governor \ + --inbox ``` Then run this to verify on Etherscan: @@ -102,7 +103,8 @@ hh verify --network mainnet \ \ \ \ - + \ + ``` ### BanxaWrapper diff --git a/addresses.json b/addresses.json index 1a527e6..b8a16c2 100644 --- a/addresses.json +++ b/addresses.json @@ -4,14 +4,14 @@ "GraphToken": "0xc944E90C64B2c07662A292be6244BDf05Cda44a7", "L1GraphTokenGateway": "0x01cDC91B0A9bA741903aA3699BF4CE31d6C5cC06", "governor": "0x74Db79268e63302d3FC69FB5a7627F7454a41732", - "BillingConnector": "0xBa321653bDb2525c866d962aD66be358F6718d78" + "BillingConnector": "0x8017B9AF3F199CC6b08A48DA3859410F20bbea72" }, "5": { "ArbitrumInbox": "0x6BEbC4925716945D46F0Ec336D5C2564F419682C", "GraphToken": "0x5c946740441C12510a167B447B7dE565C20b9E3C", "L1GraphTokenGateway": "0xc82fF7b51c3e593D709BA3dE1b3a0d233D1DEca1", "governor": "0xc6B8Dd6163a0d6E3Ff37Eb4306DeD0069FE4bF59", - "BillingConnector": "0x1c133bF1A059F682910d9bf81a09931742ec7311" + "BillingConnector": "0x0de059845a3886A10d076A34d38b6b1aB24900b2" }, "137": { "Billing": "0x10829DB618E6F520Fa3A01c75bC6dDf8722fA9fE" diff --git a/contracts/BillingConnector.sol b/contracts/BillingConnector.sol index 25c0379..ac3296d 100644 --- a/contracts/BillingConnector.sol +++ b/contracts/BillingConnector.sol @@ -154,7 +154,8 @@ contract BillingConnector is IBillingConnector, Governed, Rescuable, L1ArbitrumM } /** - * @notice Add tokens into the billing contract on L2, for any user, using a signed permit + * @notice Add tokens into the billing contract on L2 using a signed permit + * @dev _user must be the msg.sender * @param _user Address of the current owner of the tokens, that will also be the destination in L2 * @param _amount Amount of tokens to add * @param _maxGas Max gas for the L2 retryable ticket execution @@ -178,7 +179,8 @@ contract BillingConnector is IBillingConnector, Governed, Rescuable, L1ArbitrumM ) external payable override { require(_amount != 0, "Must add more than 0"); require(_user != address(0), "destination != 0"); - IERC20WithPermit(address(graphToken)).permit(_user, address(this), _amount, _deadline, _v, _r, _s); + require(_user == msg.sender, "Only tokens owner can call"); + _permit(_user, address(this), _amount, _deadline, _v, _r, _s); _addToL2(_user, _user, _amount, _maxGas, _gasPriceBid, _maxSubmissionCost); } @@ -261,4 +263,38 @@ contract BillingConnector is IBillingConnector, Governed, Rescuable, L1ArbitrumM inbox = _inbox; emit ArbitrumInboxUpdated(_inbox); } + + /** + * @dev Approve token allowance by validating a message signed by the holder, if permit + * fails check for existing allowance before reverting transaction. + * @param _owner Address of the token holder + * @param _spender Address of the approved spender + * @param _value Amount of tokens to approve the spender + * @param _deadline Expiration time of the signed permit (if zero, the permit will never expire, so use with caution) + * @param _v Signature recovery id + * @param _r Signature r value + * @param _s Signature s value + */ + function _permit( + address _owner, + address _spender, + uint256 _value, + uint256 _deadline, + uint8 _v, + bytes32 _r, + bytes32 _s + ) internal { + IERC20WithPermit token = IERC20WithPermit(address(graphToken)); + // Try permit() before allowance check to advance nonce if possible + try token.permit(_owner, _spender, _value, _deadline, _v, _r, _s) { + return; + } catch Error(string memory reason) { + // Check for existing allowance before reverting + if (token.allowance(_owner, _spender) >= _value) { + return; + } + + revert(reason); + } + } } diff --git a/package.json b/package.json index 97d2f2b..5a510ad 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "graph-studio-billing-contracts", - "version": "1.0.0", + "version": "1.0.1", "description": "Graph Studio Billing Contracts", "main": "index.js", "scripts": { diff --git a/test/billingConnector.test.ts b/test/billingConnector.test.ts index e89ab5f..2be1885 100644 --- a/test/billingConnector.test.ts +++ b/test/billingConnector.test.ts @@ -296,7 +296,7 @@ describe('BillingConnector', () => { const bridgeBalanceBefore = await token.balanceOf(l1TokenGatewayMock.address) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, oneHundred, @@ -341,13 +341,93 @@ describe('BillingConnector', () => { expect(connectorBalanceAfter).eq(connectorBalanceBefore) expect(bridgeBalanceAfter).eq(bridgeBalanceBefore.add(oneHundred)) }) + it("doesn't revert after a frontrunning permit transaction", async function () { + const permit = await permitOK(oneHundred, me.address, billingConnector.address) + const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) + + const myBalanceBefore = await token.balanceOf(me.address) + const userBalanceBefore = await token.balanceOf(user1.address) + const connectorBalanceBefore = await token.balanceOf(billingConnector.address) + const bridgeBalanceBefore = await token.balanceOf(l1TokenGatewayMock.address) + + // user2 frontruns transaction + const frontrunnerTx = await token + .connect(user2.signer) + .permit( + me.address, + billingConnector.address, + permit.value, + permit.deadline, + signedPermit.v, + signedPermit.r, + signedPermit.s, + ) + + // my original transaction with permit + const tx = await billingConnector + .connect(me.signer) + .addToL2WithPermit( + me.address, + permit.value, + defaultMaxGas, + defaultGasPriceBid, + defaultMaxSubmissionPrice, + permit.deadline, + signedPermit.v, + signedPermit.r, + signedPermit.s, + { + value: defaultMsgValue, + }, + ) + + const expectedCallhookData = defaultAbiCoder.encode(['address'], [me.address]) + const expectedOutboundCalldata = l1TokenGatewayMock.interface.encodeFunctionData('finalizeInboundTransfer', [ + token.address, + billingConnector.address, + l2BillingMock.address, + oneHundred, + defaultAbiCoder.encode(['bytes', 'bytes'], ['0x', expectedCallhookData]), + ]) + + // frontrunnerTx approves funds + await expect(frontrunnerTx) + .to.emit(token, 'Approval') + .withArgs(me.address, billingConnector.address, permit.value) + + // Transaction doesn't revert, instead it uses the allowance created by frontrunnerTx + await expect(tx).not.to.be.reverted + + // Real event emitted by BillingConnector + await expect(tx).emit(billingConnector, 'TokensSentToL2').withArgs(me.address, me.address, oneHundred) + // Mock event from the gateway mock to validate what would be sent to L2 + await expect(tx) + .emit(l1TokenGatewayMock, 'FakeTxToL2') + .withArgs( + billingConnector.address, + defaultMsgValue, + defaultMaxGas, + defaultGasPriceBid, + defaultMaxSubmissionPrice, + expectedOutboundCalldata, + ) + + // Balances are fine since transaction was executed fine + const myBalanceAfter = await token.balanceOf(me.address) + const userBalanceAfter = await token.balanceOf(user1.address) + const connectorBalanceAfter = await token.balanceOf(billingConnector.address) + const bridgeBalanceAfter = await token.balanceOf(l1TokenGatewayMock.address) + expect(myBalanceAfter).eq(myBalanceBefore.sub(oneHundred)) + expect(userBalanceAfter).eq(userBalanceBefore) // unchanged + expect(connectorBalanceAfter).eq(connectorBalanceBefore) + expect(bridgeBalanceAfter).eq(bridgeBalanceBefore.add(oneHundred)) + }) it("relies on the token's validation when the sender has provided an expired permit", async function () { - // Note the permit is from me to billingConnector, but the addToL2WithPermit tx is signed by user1 const permit = await permitExpired(me.address, billingConnector.address) const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, permit.value, @@ -369,7 +449,7 @@ describe('BillingConnector', () => { const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, permit.value, @@ -391,7 +471,7 @@ describe('BillingConnector', () => { const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, permit.value, @@ -413,7 +493,7 @@ describe('BillingConnector', () => { const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( AddressZero, permit.value, @@ -435,7 +515,7 @@ describe('BillingConnector', () => { const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, permit.value, @@ -457,7 +537,7 @@ describe('BillingConnector', () => { const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) const tx = billingConnector - .connect(user1.signer) + .connect(me.signer) .addToL2WithPermit( me.address, permit.value, @@ -474,6 +554,28 @@ describe('BillingConnector', () => { ) await expect(tx).revertedWith('WRONG_ETH_VALUE') }) + it('rejects calls if not tokens owner', async function () { + const permit = await permitOK(oneHundred, me.address, billingConnector.address) + const signedPermit = createSignedPermit(permit, myPrivateKey, SALT) + + const tx = billingConnector + .connect(user1.signer) + .addToL2WithPermit( + me.address, + permit.value, + defaultMaxGas, + defaultGasPriceBid, + defaultMaxSubmissionPrice, + permit.deadline, + signedPermit.v, + signedPermit.r, + signedPermit.s, + { + value: defaultMsgValue, + }, + ) + await expect(tx).revertedWith('Only tokens owner can call') + }) }) describe('rescueTokens', function () { it('should rescue tokens', async function () {