Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove salt from veboost domain separator and expose version #2085

Merged
merged 3 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions pkg/liquidity-mining/contracts/BoostV2.vy
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ NAME: constant(String[32]) = "Vote-Escrowed Boost"
SYMBOL: constant(String[8]) = "veBoost"
VERSION: constant(String[8]) = "v2.0.0"

EIP712_TYPEHASH: constant(bytes32) = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)")
EIP712_TYPEHASH: constant(bytes32) = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)")
PERMIT_TYPEHASH: constant(bytes32) = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)")

WEEK: constant(uint256) = 86400 * 7
Expand All @@ -73,7 +73,7 @@ migrated: public(HashMap[uint256, bool])
@external
def __init__(_boost_v1: address, _ve: address):
BOOST_V1 = _boost_v1
DOMAIN_SEPARATOR = keccak256(_abi_encode(EIP712_TYPEHASH, keccak256(NAME), keccak256(VERSION), chain.id, self, block.prevhash))
DOMAIN_SEPARATOR = keccak256(_abi_encode(EIP712_TYPEHASH, keccak256(NAME), keccak256(VERSION), chain.id, self))
VE = _ve

log Transfer(ZERO_ADDRESS, msg.sender, 0)
Expand Down Expand Up @@ -269,8 +269,7 @@ def approve(_spender: address, _value: uint256) -> bool:

@external
def permit(_owner: address, _spender: address, _value: uint256, _deadline: uint256, _v: uint8, _r: bytes32, _s: bytes32) -> bool:
assert _owner != ZERO_ADDRESS
assert block.timestamp <= _deadline
assert block.timestamp <= _deadline, 'EXPIRED_SIGNATURE'

nonce: uint256 = self.nonces[_owner]
digest: bytes32 = keccak256(
Expand All @@ -281,7 +280,7 @@ def permit(_owner: address, _spender: address, _value: uint256, _deadline: uint2
)
)

assert ecrecover(digest, convert(_v, uint256), convert(_r, uint256), convert(_s, uint256)) == _owner
assert ecrecover(digest, convert(_v, uint256), convert(_r, uint256), convert(_s, uint256)) == _owner and _owner != ZERO_ADDRESS, 'INVALID_SIGNATURE'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call


self.allowance[_owner][_spender] = _value
self.nonces[_owner] = nonce + 1
Expand Down Expand Up @@ -370,6 +369,10 @@ def decimals() -> uint8:
def BOOST_V1() -> address:
return BOOST_V1

@pure
@external
def version() -> String[8]:
return VERSION

@pure
@external
Expand Down
131 changes: 131 additions & 0 deletions pkg/liquidity-mining/test/BoostV2.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { BigNumber, Contract } from 'ethers';
import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/dist/src/signer-with-address';

import * as expectEvent from '@balancer-labs/v2-helpers/src/test/expectEvent';
import { deploy } from '@balancer-labs/v2-helpers/src/contract';
import { MAX_UINT256 as MAX_DEADLINE, ZERO_ADDRESS } from '@balancer-labs/v2-helpers/src/constants';
import { bn } from '@balancer-labs/v2-helpers/src/numbers';
import { signPermit } from '@balancer-labs/balancer-js';
import { currentTimestamp } from '@balancer-labs/v2-helpers/src/time';

describe('BoostV2', () => {
let boost: Contract;
let holder: SignerWithAddress, spender: SignerWithAddress;

before('setup signers', async () => {
[, holder, spender] = await ethers.getSigners();
});

sharedBeforeEach('deploy veBoostV2', async () => {
boost = await deploy('BoostV2', { args: [ZERO_ADDRESS, ZERO_ADDRESS] });
});

describe('info', () => {
it('sets up the name properly', async () => {
expect(await boost.name()).to.be.equal('Vote-Escrowed Boost');
});

it('sets up the symbol properly', async () => {
expect(await boost.symbol()).to.be.equal('veBoost');
});

it('sets up the version properly', async () => {
expect(await boost.version()).to.be.equal('v2.0.0');
});
});

describe('permit', () => {
it('initial nonce is zero', async () => {
expect(await boost.nonces(holder.address)).to.equal(0);
});

const amount = bn(42);

it('accepts holder signature', async function () {
const previousNonce = await boost.nonces(holder.address);
const { v, r, s } = await signPermit(boost, holder, spender, amount);

const receipt = await (await boost.permit(holder.address, spender.address, amount, MAX_DEADLINE, v, r, s)).wait();
expectEvent.inReceipt(receipt, 'Approval', { _owner: holder.address, _spender: spender.address, _value: amount });

expect(await boost.nonces(holder.address)).to.equal(previousNonce.add(1));
expect(await boost.allowance(holder.address, spender.address)).to.equal(amount);
});

context('with invalid signature', () => {
let v: number, r: string, s: string, deadline: BigNumber;

context('with reused signature', () => {
beforeEach(async () => {
({ v, r, s, deadline } = await signPermit(boost, holder, spender, amount));
await boost.permit(holder.address, spender.address, amount, deadline, v, r, s);
});

itRevertsWithInvalidSignature();
});

context('with signature for other holder', () => {
beforeEach(async () => {
({ v, r, s, deadline } = await signPermit(boost, spender, spender, amount));
});

itRevertsWithInvalidSignature();
});

context('with signature for other spender', () => {
beforeEach(async () => {
({ v, r, s, deadline } = await signPermit(boost, holder, holder, amount));
});

itRevertsWithInvalidSignature();
});

context('with signature for other amount', () => {
beforeEach(async () => {
({ v, r, s, deadline } = await signPermit(boost, holder, spender, amount.add(1)));
});

itRevertsWithInvalidSignature();
});

context('with signature for other token', () => {
beforeEach(async () => {
const otherToken = await deploy('v2-solidity-utils/ERC20PermitMock', { args: ['Token', 'TKN'] });

({ v, r, s, deadline } = await signPermit(otherToken, holder, spender, amount));
});

itRevertsWithInvalidSignature();
});

context('with signature with invalid nonce', () => {
beforeEach(async () => {
const currentNonce = await boost.nonces(holder.address);
({ v, r, s, deadline } = await signPermit(boost, holder, spender, amount, MAX_DEADLINE, currentNonce.add(1)));
});

itRevertsWithInvalidSignature();
});

context('with expired deadline', () => {
beforeEach(async () => {
const now = await currentTimestamp();

({ v, r, s, deadline } = await signPermit(boost, holder, spender, amount, now.sub(1)));
});

itRevertsWithInvalidSignature('EXPIRED_SIGNATURE');
});

function itRevertsWithInvalidSignature(reason?: string) {
it('reverts', async () => {
await expect(boost.permit(holder.address, spender.address, amount, deadline, v, r, s)).to.be.revertedWith(
reason ?? 'INVALID_SIGNATURE'
);
});
}
});
});
});