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.

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

Jump to bottom

Validate the POI is from the canonical chain and close to chain head #506

Closed
wants to merge 5 commits into from
Closed

Validate the POI is from the canonical chain and close to chain head #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c717b7c
staking: use multicall on the Staking contract
abarmat Jul 19, 2021
93927e5
staking: validate the POI is from the canonical chain and fresh
abarmat Sep 23, 2021
44e9f1d
staking: cleanup and fix tests
abarmat Sep 29, 2021
c7d34c6
staking: set valid poi range as a constant
abarmat Sep 30, 2021
2c728cd
staking: add tests
abarmat Oct 5, 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
33 changes: 10 additions & 23 deletions contracts/staking/IStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ interface IStaking is IStakingData {
* - Finalized = Closed && closedAtEpoch + channelDisputeEpochs > now()
* - Claimed = not Null && tokens == 0
*/
enum AllocationState { Null, Active, Closed, Finalized, Claimed }
enum AllocationState {
Null,
Active,
Closed,
Finalized,
Claimed
}

// -- Configuration --

Expand Down Expand Up @@ -88,14 +94,6 @@ interface IStaking is IStakingData {

// -- Channel management and allocations --

function allocate(
bytes32 _subgraphDeploymentID,
uint256 _tokens,
address _allocationID,
bytes32 _metadata,
bytes calldata _proof
) external;

function allocateFrom(
address _indexer,
bytes32 _subgraphDeploymentID,
Expand All @@ -105,27 +103,16 @@ interface IStaking is IStakingData {
bytes calldata _proof
) external;

function closeAllocation(address _allocationID, bytes32 _poi) external;

function closeAllocationMany(CloseAllocationRequest[] calldata _requests) external;

function closeAndAllocate(
address _oldAllocationID,
bytes32 _poi,
address _indexer,
bytes32 _subgraphDeploymentID,
uint256 _tokens,
function closeAllocation(
address _allocationID,
bytes32 _metadata,
bytes calldata _proof
bytes32 _poi,
bytes32 _poiBlockHash
) external;

function collect(uint256 _tokens, address _allocationID) external;

function claim(address _allocationID, bool _restake) external;

function claimMany(address[] calldata _allocationID, bool _restake) external;

// -- Getters and calculations --

function hasStake(address _indexer) external view returns (bool);
Expand Down
114 changes: 36 additions & 78 deletions contracts/staking/Staking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/cryptography/ECDSA.sol";

import "../base/Multicall.sol";
import "../upgrades/GraphUpgradeable.sol";
import "../utils/TokenUtils.sol";

Expand All @@ -20,13 +21,15 @@ import "./libs/Stakes.sol";
* Allocations on a Subgraph. It also allows Delegators to Delegate towards an Indexer. The
* contract also has the slashing functionality.
*/
contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
using SafeMath for uint256;
using Stakes for Stakes.Indexer;
using Rebates for Rebates.Pool;

// 100% in parts per million
uint32 private constant MAX_PPM = 1000000;
// Range to use for valid POI blocks from latest block number
uint256 private constant POI_VALID_BLOCK_RANGE = 256;
Copy link

Choose a reason for hiding this comment

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

This is only ~ an hour - given that the PoI needs to be for the first block in an epoch, does this mean that indexers will only be able to close allocations in the first hour of the epoch (or it won't be able to find the blockhash)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Arbitration Charter needs to change to allow any indexer to be able to provide a proof within the N amount of blocks from the block they sent the transaction instead of using the start block of epoch.


// -- Events --

Expand Down Expand Up @@ -145,6 +148,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
uint256 effectiveAllocation,
address sender,
bytes32 poi,
bytes32 poiBlockHash,
bool isDelegator
);

Expand Down Expand Up @@ -858,24 +862,6 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
return _withdrawDelegated(msg.sender, _indexer, _delegateToIndexer);
}

/**
* @dev Allocate available tokens to a subgraph deployment.
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
* @param _tokens Amount of tokens to allocate
* @param _allocationID The allocation identifier
* @param _metadata IPFS hash for additional information about the allocation
* @param _proof A 65-bytes Ethereum signed message of `keccak256(indexerAddress,allocationID)`
*/
function allocate(
bytes32 _subgraphDeploymentID,
uint256 _tokens,
address _allocationID,
bytes32 _metadata,
bytes calldata _proof
) external override notPaused {
_allocate(msg.sender, _subgraphDeploymentID, _tokens, _allocationID, _metadata, _proof);
}

/**
* @dev Allocate available tokens to a subgraph deployment.
* @param _indexer Indexer address to allocate funds from.
Expand Down Expand Up @@ -903,52 +889,14 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
* To opt out for rewards set _poi to 0x0
* @param _allocationID The allocation identifier
* @param _poi Proof of indexing submitted for the allocated period
* @param _poiBlockHash Blockhash for which the POI was calculated
*/
function closeAllocation(address _allocationID, bytes32 _poi) external override notPaused {
_closeAllocation(_allocationID, _poi);
}

/**
* @dev Close multiple allocations and free the staked tokens.
* To be eligible for rewards a proof of indexing must be presented.
* Presenting a bad proof is subject to slashable condition.
* To opt out for rewards set _poi to 0x0
* @param _requests An array of CloseAllocationRequest
*/
function closeAllocationMany(CloseAllocationRequest[] calldata _requests)
external
override
notPaused
{
for (uint256 i = 0; i < _requests.length; i++) {
_closeAllocation(_requests[i].allocationID, _requests[i].poi);
}
}

/**
* @dev Close and allocate. This will perform a close and then create a new Allocation
* atomically on the same transaction.
* @param _closingAllocationID The identifier of the allocation to be closed
* @param _poi Proof of indexing submitted for the allocated period
* @param _indexer Indexer address to allocate funds from.
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
* @param _tokens Amount of tokens to allocate
* @param _allocationID The allocation identifier
* @param _metadata IPFS hash for additional information about the allocation
* @param _proof A 65-bytes Ethereum signed message of `keccak256(indexerAddress,allocationID)`
*/
function closeAndAllocate(
address _closingAllocationID,
bytes32 _poi,
address _indexer,
bytes32 _subgraphDeploymentID,
uint256 _tokens,
function closeAllocation(
address _allocationID,
bytes32 _metadata,
bytes calldata _proof
bytes32 _poi,
bytes32 _poiBlockHash
) external override notPaused {
_closeAllocation(_closingAllocationID, _poi);
_allocate(_indexer, _subgraphDeploymentID, _tokens, _allocationID, _metadata, _proof);
_closeAllocation(_allocationID, _poi, _poiBlockHash);
}

/**
Expand Down Expand Up @@ -1036,21 +984,6 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
_claim(_allocationID, _restake);
}

/**
* @dev Claim tokens from the rebate pool for many allocations.
* @param _allocationID Array of allocations from where we are claiming tokens
* @param _restake True if restake fees instead of transfer to indexer
*/
function claimMany(address[] calldata _allocationID, bool _restake)
external
override
notPaused
{
for (uint256 i = 0; i < _allocationID.length; i++) {
_claim(_allocationID[i], _restake);
}
}

/**
* @dev Stake tokens on the indexer.
* This function does not check minimum indexer stake requirement to allow
Expand Down Expand Up @@ -1155,19 +1088,43 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
);
}

/**
* @dev Check if the passed blockhash is part of the canonical chain.
* @param _blockHash An Ethereum block hash
*/
function _checkValidBlockHash(bytes32 _blockHash) private view returns (bool) {
uint256 currentBlockNum = block.number;
for (uint256 i = 0; i < POI_VALID_BLOCK_RANGE; i++) {
if (_blockHash == blockhash(currentBlockNum - i)) {
return true;
}
}
return false;
}

/**
* @dev Close an allocation and free the staked tokens.
* @param _allocationID The allocation identifier
* @param _poi Proof of indexing submitted for the allocated period
* @param _poiBlockHash Blockhash for which the POI was calculated
*/
function _closeAllocation(address _allocationID, bytes32 _poi) private {
function _closeAllocation(
address _allocationID,
bytes32 _poi,
bytes32 _poiBlockHash
) private {
// Allocation must exist and be active
AllocationState allocState = _getAllocationState(_allocationID);
require(allocState == AllocationState.Active, "!active");

// Get allocation
Allocation memory alloc = allocations[_allocationID];

// Block validation for the POI
if (_poi != 0) {
require(_poiBlockHash > 0 && _checkValidBlockHash(_poiBlockHash), "!poi-block");
}

// Validate that an allocation cannot be closed before one epoch
alloc.closedAtEpoch = epochManager().currentEpoch();
uint256 epochs = MathUtils.diffOrZero(alloc.closedAtEpoch, alloc.createdAtEpoch);
Expand Down Expand Up @@ -1226,6 +1183,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
alloc.effectiveAllocation,
msg.sender,
_poi,
_poiBlockHash,
!isIndexer
);
}
Expand Down
11 changes: 8 additions & 3 deletions test/disputes/poi.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
toBN,
toGRT,
Account,
latestBlockHash,
} from '../lib/testHelpers'

import { MAX_PPM } from './common'
Expand Down Expand Up @@ -79,7 +80,8 @@ describe('DisputeManager:POI', async () => {
await staking.connect(indexerAccount.signer).stake(indexerTokens)
await staking
.connect(indexerAccount.signer)
.allocate(
.allocateFrom(
indexerAccount.address,
subgraphDeploymentID,
indexerAllocatedTokens,
allocationID,
Expand Down Expand Up @@ -147,7 +149,8 @@ describe('DisputeManager:POI', async () => {
await staking.connect(indexer.signer).stake(indexerTokens)
const tx1 = await staking
.connect(indexer.signer)
.allocate(
.allocateFrom(
indexer.address,
subgraphDeploymentID,
indexerAllocatedTokens,
allocationID,
Expand All @@ -158,7 +161,9 @@ describe('DisputeManager:POI', async () => {
const event1 = staking.interface.parseLog(receipt1.logs[0]).args
await advanceToNextEpoch(epochManager) // wait the required one epoch to close allocation
await staking.connect(assetHolder.signer).collect(indexerCollectedTokens, event1.allocationID)
await staking.connect(indexer.signer).closeAllocation(event1.allocationID, poi)
await staking
.connect(indexer.signer)
.closeAllocation(event1.allocationID, poi, await latestBlockHash())
await staking.connect(indexer.signer).unstake(indexerTokens)
await advanceBlock() // pass thawing period
await staking.connect(indexer.signer).withdraw()
Expand Down
11 changes: 8 additions & 3 deletions test/disputes/query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
toBN,
toGRT,
Account,
latestBlockHash,
} from '../lib/testHelpers'

import { Dispute, createQueryDisputeID, encodeAttestation, MAX_PPM } from './common'
Expand Down Expand Up @@ -113,7 +114,8 @@ describe('DisputeManager:Query', async () => {
await staking.connect(indexerAccount.signer).stake(indexerTokens)
await staking
.connect(indexerAccount.signer)
.allocate(
.allocateFrom(
indexerAccount.address,
dispute.receipt.subgraphDeploymentID,
indexerAllocatedTokens,
allocationID,
Expand Down Expand Up @@ -194,7 +196,8 @@ describe('DisputeManager:Query', async () => {
await staking.connect(indexer.signer).stake(indexerTokens)
const tx1 = await staking
.connect(indexer.signer)
.allocate(
.allocateFrom(
indexer.address,
dispute.receipt.subgraphDeploymentID,
indexerAllocatedTokens,
indexer1ChannelKey.address,
Expand All @@ -205,7 +208,9 @@ describe('DisputeManager:Query', async () => {
const event1 = staking.interface.parseLog(receipt1.logs[0]).args
await advanceToNextEpoch(epochManager) // wait the required one epoch to close allocation
await staking.connect(assetHolder.signer).collect(indexerCollectedTokens, event1.allocationID)
await staking.connect(indexer.signer).closeAllocation(event1.allocationID, poi)
await staking
.connect(indexer.signer)
.closeAllocation(event1.allocationID, poi, await latestBlockHash())
await staking.connect(indexer.signer).unstake(indexerTokens)
await advanceBlock() // pass thawing period
await staking.connect(indexer.signer).withdraw()
Expand Down
5 changes: 5 additions & 0 deletions test/lib/testHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ export const getChainID = (): Promise<number> => {
export const latestBlock = (): Promise<BigNumber> =>
provider().send('eth_blockNumber', []).then(toBN)

export const latestBlockHash = (): Promise<string> =>
provider()
.getBlock('latest')
.then((b) => b.hash)

export const advanceBlock = (): Promise<void> => {
return provider().send('evm_mine', [])
}
Expand Down
3 changes: 2 additions & 1 deletion test/payments/allocationExchange.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ describe('AllocationExchange', () => {
await staking.connect(indexer.signer).stake(stakeTokens)
await staking
.connect(indexer.signer)
.allocate(
.allocateFrom(
indexer.address,
subgraphDeploymentID,
stakeTokens,
allocationID,
Expand Down
3 changes: 2 additions & 1 deletion test/payments/withdrawHelper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ describe('WithdrawHelper', () => {
await staking.connect(indexer.signer).stake(stakeTokens)
await staking
.connect(indexer.signer)
.allocate(
.allocateFrom(
indexer.address,
subgraphDeploymentID,
stakeTokens,
allocationID,
Expand Down
Loading