Skip to content

[AllocationExchange:H01] Support multiple voucher signers #480

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 16, 2021
Merged

[AllocationExchange:H01] Support multiple voucher signers #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
721e706
Support multiple voucher signers
abarmat Jun 27, 2021
612eaab
Merge branch 'dev' into audit/allocation-exchange-h01
abarmat Jul 16, 2021
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
26 changes: 12 additions & 14 deletions contracts/statechannels/AllocationExchange.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@ contract AllocationExchange is Governed {

IStaking private immutable staking;
IGraphToken private immutable graphToken;
address public authority;

// Tracks redeemed allocationIDs. An allocation can only be redeemed once.
mapping(address => bool) public authority;
mapping(address => bool) public allocationsRedeemed;

// -- Events

event AuthoritySet(address indexed account);
event AuthoritySet(address indexed account, bool authorized);
event AllocationRedeemed(address indexed allocationID, uint256 amount);
event TokensWithdrawn(address indexed to, uint256 amount);

Expand All @@ -69,7 +67,7 @@ contract AllocationExchange is Governed {

graphToken = _graphToken;
staking = _staking;
_setAuthority(_authority);
_setAuthority(_authority, true);
}

/**
Expand Down Expand Up @@ -97,25 +95,27 @@ contract AllocationExchange is Governed {
* @notice Set the authority allowed to sign vouchers.
* @dev Only the governor can set the authority
* @param _authority Address of the signing authority
* @param _authorized True if the authority is authorized to sign vouchers, false to unset
*/
function setAuthority(address _authority) external onlyGovernor {
_setAuthority(_authority);
function setAuthority(address _authority, bool _authorized) external onlyGovernor {
_setAuthority(_authority, _authorized);
}

/**
* @notice Set the authority allowed to sign vouchers.
* @param _authority Address of the signing authority
* @param _authorized True if the authority is authorized to sign vouchers, false to unset
*/
function _setAuthority(address _authority) private {
function _setAuthority(address _authority, bool _authorized) private {
require(_authority != address(0), "Exchange: empty authority");
// This will help catch some operational errors but not all.
// The validation will fail under the following conditions:
// - a contract in construction
// - an address where a contract will be created
// - an address where a contract lived, but was destroyed
require(!Address.isContract(_authority), "Exchange: authority must be EOA");
authority = _authority;
emit AuthoritySet(authority);
authority[_authority] = _authorized;
emit AuthoritySet(_authority, _authorized);
}

/**
Expand Down Expand Up @@ -155,10 +155,8 @@ contract AllocationExchange is Governed {

// Signature check
bytes32 messageHash = keccak256(abi.encodePacked(_voucher.allocationID, _voucher.amount));
require(
authority == ECDSA.recover(messageHash, _voucher.signature),
"Exchange: invalid signer"
);
address voucherSigner = ECDSA.recover(messageHash, _voucher.signature);
require(authority[voucherSigner], "Exchange: invalid signer");

// Mark allocation as collected
allocationsRedeemed[_voucher.allocationID] = true;
Expand Down
20 changes: 13 additions & 7 deletions test/payments/allocationExchange.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('AllocationExchange', () => {

// Ensure the exchange is correctly setup
await staking.connect(governor.signer).setAssetHolder(allocationExchange.address, true)
await allocationExchange.connect(governor.signer).setAuthority(authority.address)
await allocationExchange.connect(governor.signer).setAuthority(authority.address, true)
await allocationExchange.approveAll()
})

Expand Down Expand Up @@ -114,21 +114,27 @@ describe('AllocationExchange', () => {

describe('config', function () {
it('should set an authority', async function () {
// Set authority
const newAuthority = randomAddress()
const tx = allocationExchange.connect(governor.signer).setAuthority(newAuthority)
await expect(tx).emit(allocationExchange, 'AuthoritySet').withArgs(newAuthority)
expect(await allocationExchange.authority()).eq(newAuthority)
const tx1 = allocationExchange.connect(governor.signer).setAuthority(newAuthority, true)
await expect(tx1).emit(allocationExchange, 'AuthoritySet').withArgs(newAuthority, true)
expect(await allocationExchange.authority(newAuthority)).eq(true)

// Unset authority
const tx2 = allocationExchange.connect(governor.signer).setAuthority(newAuthority, false)
await expect(tx2).emit(allocationExchange, 'AuthoritySet').withArgs(newAuthority, false)
expect(await allocationExchange.authority(newAuthority)).eq(false)
})

it('reject set an authority if not allowed', async function () {
const newAuthority = randomAddress()
const tx = allocationExchange.connect(indexer.signer).setAuthority(newAuthority)
const tx = allocationExchange.connect(indexer.signer).setAuthority(newAuthority, true)
await expect(tx).revertedWith(' Only Governor can call')
})

it('reject set an empty authority', async function () {
const newAuthority = AddressZero
const tx = allocationExchange.connect(governor.signer).setAuthority(newAuthority)
const tx = allocationExchange.connect(governor.signer).setAuthority(newAuthority, true)
await expect(tx).revertedWith('Exchange: empty authority')
})

Expand Down Expand Up @@ -225,7 +231,7 @@ describe('AllocationExchange', () => {
const allocationID = '0xfefefefefefefefefefefefefefefefefefefefe'

// Ensure the exchange is correctly setup
await allocationExchange.connect(governor.signer).setAuthority(authority.address)
await allocationExchange.connect(governor.signer).setAuthority(authority.address, true)
await allocationExchange.approveAll()

// Initiate a withdrawal
Expand Down