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

feat: remove asset holder allowlist, with additional fixes #864

Merged
merged 21 commits into from
Dec 7, 2023
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bcf31e9
feat: remove asset holder allowlist
pcarranzav Nov 15, 2022
2e8c499
fix: remove setAssetHolder call from config files
tmigone Nov 16, 2022
36308c3
fix: a few details for asset holder deprecation
pcarranzav Nov 24, 2022
b56af5d
fix: improvements from Trust audit (TRST-L-1 and recommendations)
pcarranzav Feb 17, 2023
225be78
test: fix the case when collecting zero fees
pcarranzav Feb 17, 2023
c2fdebb
Merge pull request #791 from graphprotocol/pcv/asset-holder-trust-fixes
pcarranzav Feb 23, 2023
765d43f
chore: add Trust audit report
pcarranzav Feb 23, 2023
194fb4f
Merge branch 'dev' into pcv/remove-asset-holder-check
pcarranzav Aug 2, 2023
04a42bc
fix: remove assetHolders getter
pcarranzav Aug 3, 2023
4b334cb
test: remove a leftover setAssetHolder call
pcarranzav Aug 3, 2023
d3b626c
fix: use a bracket scope to avoid stack too deep
pcarranzav Sep 5, 2023
c090c63
fix: typo
pcarranzav Sep 5, 2023
abe55a1
test: fix a case where it still expected an event
pcarranzav Sep 6, 2023
bf1aa37
fix: require that an allocation is open for an epoch before collecting
pcarranzav Oct 11, 2023
22e838f
fix: enforce a minimum of 1 GRT when delegating to an indexer for the…
pcarranzav Oct 11, 2023
ef55d45
fix: enforce minimum delegation when receiving from L1
pcarranzav Oct 11, 2023
3882b35
fix: enforce minimum delegation in all cases, including undelegation
pcarranzav Oct 12, 2023
f9af7eb
fix: round up when calculating curation fees and the protocol tax (OZ…
pcarranzav Nov 9, 2023
821c93a
fix: clean up the check for remaining delegation (OZ N-01)
pcarranzav Nov 10, 2023
842445d
fix: add a rounding error protection when receiving subgraphs or sign…
pcarranzav Nov 22, 2023
afe7f9f
fix: add comment on not being able to revert
pcarranzav Nov 24, 2023
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
Binary file not shown.
3 changes: 0 additions & 3 deletions config/graph.arbitrum-goerli.yml
Original file line number Diff line number Diff line change
@@ -123,9 +123,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
3 changes: 0 additions & 3 deletions config/graph.arbitrum-localhost.yml
Original file line number Diff line number Diff line change
@@ -123,9 +123,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
3 changes: 0 additions & 3 deletions config/graph.arbitrum-one.yml
Original file line number Diff line number Diff line change
@@ -123,9 +123,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
3 changes: 0 additions & 3 deletions config/graph.goerli.yml
Original file line number Diff line number Diff line change
@@ -126,9 +126,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
3 changes: 0 additions & 3 deletions config/graph.localhost.yml
Original file line number Diff line number Diff line change
@@ -131,9 +131,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
3 changes: 0 additions & 3 deletions config/graph.mainnet.yml
Original file line number Diff line number Diff line change
@@ -126,9 +126,6 @@ contracts:
- fn: "setSlasher"
slasher: "${{DisputeManager.address}}"
allowed: true
- fn: "setAssetHolder"
assetHolder: "${{AllocationExchange.address}}"
allowed: true
- fn: "syncAllContracts"
RewardsManager:
proxy: true
13 changes: 13 additions & 0 deletions contracts/l2/curation/IL2Curation.sol
Original file line number Diff line number Diff line change
@@ -29,4 +29,17 @@ interface IL2Curation {
external
view
returns (uint256);

/**
* @notice Calculate the amount of tokens that would be recovered if minting signal with
* the input tokens and then burning it. This can be used to compute rounding error.
* This function does not account for curation tax.
* @param _subgraphDeploymentID Subgraph deployment for which to mint signal
* @param _tokensIn Amount of tokens used to mint signal
* @return Amount of tokens that would be recovered after minting and burning signal
*/
function tokensToSignalToTokensNoTax(bytes32 _subgraphDeploymentID, uint256 _tokensIn)
external
view
returns (uint256);
}
22 changes: 22 additions & 0 deletions contracts/l2/curation/L2Curation.sol
Original file line number Diff line number Diff line change
@@ -421,6 +421,28 @@ contract L2Curation is CurationV2Storage, GraphUpgradeable, IL2Curation {
return _tokensToSignal(_subgraphDeploymentID, _tokensIn);
}

/**
* @notice Calculate the amount of tokens that would be recovered if minting signal with
* the input tokens and then burning it. This can be used to compute rounding error.
* This function does not account for curation tax.
* @param _subgraphDeploymentID Subgraph deployment for which to mint signal
* @param _tokensIn Amount of tokens used to mint signal
* @return Amount of tokens that would be recovered after minting and burning signal
*/
function tokensToSignalToTokensNoTax(bytes32 _subgraphDeploymentID, uint256 _tokensIn)
Copy link
Contributor

Choose a reason for hiding this comment

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

lol @ that name

Copy link
Member Author

Choose a reason for hiding this comment

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

tokensToSignalToTokensNoTaxFinalFinalVersion4.docx

external
view
override
returns (uint256)
{
require(_tokensIn != 0, "Can't calculate with 0 tokens");
uint256 signal = _tokensToSignal(_subgraphDeploymentID, _tokensIn);
CurationPool memory curationPool = pools[_subgraphDeploymentID];
uint256 poolSignalAfter = getCurationPoolSignal(_subgraphDeploymentID).add(signal);
uint256 poolTokensAfter = curationPool.tokens.add(_tokensIn);
return poolTokensAfter.mul(signal).div(poolSignalAfter);
}

/**
* @notice Calculate number of tokens to get when burning signal from a curation pool.
* @param _subgraphDeploymentID Subgraph deployment for which to burn signal
7 changes: 7 additions & 0 deletions contracts/l2/discovery/IL2GNS.sol
Original file line number Diff line number Diff line change
@@ -46,4 +46,11 @@ interface IL2GNS is ICallhookReceiver {
* @return L2 subgraph ID
*/
function getAliasedL2SubgraphID(uint256 _l1SubgraphID) external pure returns (uint256);

/**
* @notice Return the unaliased L1 subgraph ID from a transferred L2 subgraph ID
* @param _l2SubgraphID L2 subgraph ID
* @return L1subgraph ID
*/
function getUnaliasedL1SubgraphID(uint256 _l2SubgraphID) external pure returns (uint256);
}
76 changes: 62 additions & 14 deletions contracts/l2/discovery/L2GNS.sol
Original file line number Diff line number Diff line change
@@ -26,9 +26,17 @@ import { IL2Curation } from "../curation/IL2Curation.sol";
contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
using SafeMathUpgradeable for uint256;

/// Offset added to an L1 subgraph ID to compute the L2 subgraph ID alias
uint256 public constant SUBGRAPH_ID_ALIAS_OFFSET =
uint256(0x1111000000000000000000000000000000000000000000000000000000001111);

/// Maximum rounding error when receiving signal tokens from L1, in parts-per-million.
/// If the error from minting signal is above this, tokens will be sent back to the curator.
uint256 public constant MAX_ROUNDING_ERROR = 1000;

/// @dev 100% expressed in parts-per-million
uint256 private constant MAX_PPM = 1000000;

/// @dev Emitted when a subgraph is received from L1 through the bridge
event SubgraphReceivedFromL1(
uint256 indexed _l1SubgraphID,
@@ -122,10 +130,34 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
require(_subgraphDeploymentID != 0, "GNS: deploymentID != 0");

IL2Curation curation = IL2Curation(address(curation()));
// Update pool: constant nSignal, vSignal can change (w/no slippage protection)
// Buy all signal from the new deployment
uint256 vSignal = curation.mintTaxFree(_subgraphDeploymentID, transferData.tokens);
uint256 nSignal = vSignalToNSignal(_l2SubgraphID, vSignal);

uint256 vSignal;
uint256 nSignal;
uint256 roundingError;
uint256 tokens = transferData.tokens;
{
// This can't revert because the bridge ensures that _tokensIn is > 0,
// and the minimum curation in L2 is 1 wei GRT
uint256 tokensAfter = curation.tokensToSignalToTokensNoTax(
_subgraphDeploymentID,
tokens
);
roundingError = tokens.sub(tokensAfter).mul(MAX_PPM).div(tokens);
}
if (roundingError <= MAX_ROUNDING_ERROR) {
vSignal = curation.mintTaxFree(_subgraphDeploymentID, tokens);
nSignal = vSignalToNSignal(_l2SubgraphID, vSignal);
emit SignalMinted(_l2SubgraphID, msg.sender, nSignal, vSignal, tokens);
emit SubgraphUpgraded(_l2SubgraphID, vSignal, tokens, _subgraphDeploymentID);
} else {
graphToken().transfer(msg.sender, tokens);
emit CuratorBalanceReturnedToBeneficiary(
getUnaliasedL1SubgraphID(_l2SubgraphID),
msg.sender,
tokens
);
emit SubgraphUpgraded(_l2SubgraphID, vSignal, 0, _subgraphDeploymentID);
}

subgraphData.disabled = false;
subgraphData.vSignal = vSignal;
@@ -134,16 +166,8 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
subgraphData.subgraphDeploymentID = _subgraphDeploymentID;
// Set the token metadata
_setSubgraphMetadata(_l2SubgraphID, _subgraphMetadata);

emit SubgraphPublished(_l2SubgraphID, _subgraphDeploymentID, fixedReserveRatio);
emit SubgraphUpgraded(
_l2SubgraphID,
subgraphData.vSignal,
transferData.tokens,
_subgraphDeploymentID
);
emit SubgraphVersionUpdated(_l2SubgraphID, _subgraphDeploymentID, _versionMetadata);
emit SignalMinted(_l2SubgraphID, msg.sender, nSignal, vSignal, transferData.tokens);
emit SubgraphL2TransferFinalized(_l2SubgraphID);
}

@@ -227,6 +251,20 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
return _l1SubgraphID + SUBGRAPH_ID_ALIAS_OFFSET;
}

/**
* @notice Return the unaliased L1 subgraph ID from a transferred L2 subgraph ID
* @param _l2SubgraphID L2 subgraph ID
* @return L1subgraph ID
*/
function getUnaliasedL1SubgraphID(uint256 _l2SubgraphID)
public
pure
override
returns (uint256)
{
return _l2SubgraphID - SUBGRAPH_ID_ALIAS_OFFSET;
}

/**
* @dev Receive a subgraph from L1.
* This function will initialize a subgraph received through the bridge,
@@ -278,13 +316,23 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
IL2GNS.SubgraphL2TransferData storage transferData = subgraphL2TransferData[l2SubgraphID];
SubgraphData storage subgraphData = _getSubgraphData(l2SubgraphID);

IL2Curation curation = IL2Curation(address(curation()));
uint256 roundingError;
if (transferData.l2Done && !subgraphData.disabled) {
// This can't revert because the bridge ensures that _tokensIn is > 0,
// and the minimum curation in L2 is 1 wei GRT
uint256 tokensAfter = curation.tokensToSignalToTokensNoTax(
subgraphData.subgraphDeploymentID,
_tokensIn
);
roundingError = _tokensIn.sub(tokensAfter).mul(MAX_PPM).div(_tokensIn);
}
// If subgraph transfer wasn't finished, we should send the tokens to the curator
if (!transferData.l2Done || subgraphData.disabled) {
if (!transferData.l2Done || subgraphData.disabled || roundingError > MAX_ROUNDING_ERROR) {
graphToken().transfer(_curator, _tokensIn);
emit CuratorBalanceReturnedToBeneficiary(_l1SubgraphID, _curator, _tokensIn);
} else {
// Get name signal to mint for tokens deposited
IL2Curation curation = IL2Curation(address(curation()));
uint256 vSignal = curation.mintTaxFree(subgraphData.subgraphDeploymentID, _tokensIn);
uint256 nSignal = vSignalToNSignal(l2SubgraphID, vSignal);

9 changes: 7 additions & 2 deletions contracts/l2/staking/L2Staking.sol
Original file line number Diff line number Diff line change
@@ -19,6 +19,9 @@ contract L2Staking is Staking, IL2StakingBase {
using SafeMath for uint256;
using Stakes for Stakes.Indexer;

/// @dev Minimum amount of tokens that can be delegated
uint256 private constant MINIMUM_DELEGATION = 1e18;

/**
* @dev Emitted when `delegator` delegated `tokens` to the `indexer`, the delegator
* gets `shares` for the delegation pool proportionally to the tokens staked.
@@ -124,8 +127,10 @@ contract L2Staking is Staking, IL2StakingBase {
// Calculate shares to issue (without applying any delegation tax)
uint256 shares = (pool.tokens == 0) ? _amount : _amount.mul(pool.shares).div(pool.tokens);

if (shares == 0) {
// If no shares would be issued (probably a rounding issue or attack), return the tokens to the delegator
if (shares == 0 || _amount < MINIMUM_DELEGATION) {
// If no shares would be issued (probably a rounding issue or attack),
// or if the amount is under the minimum delegation (which could be part of a rounding attack),
// return the tokens to the delegator
graphToken().transfer(_delegationData.delegator, _amount);
emit TransferredDelegationReturnedToDelegator(
_delegationData.indexer,
14 changes: 0 additions & 14 deletions contracts/staking/IStakingBase.sol
Original file line number Diff line number Diff line change
@@ -93,12 +93,6 @@ interface IStakingBase is IStakingData {
uint32 cooldownBlocks
);

/**
* @dev Emitted when `caller` set `assetHolder` address as `allowed` to send funds
* to staking contract.
*/
event AssetHolderUpdate(address indexed caller, address indexed assetHolder, bool allowed);

/**
* @dev Emitted when `indexer` set `operator` access.
*/
@@ -219,14 +213,6 @@ interface IStakingBase is IStakingData {
uint32 _lambdaDenominator
) external;

/**
* @notice Set an address as allowed asset holder.
* @dev This function can only be called by the governor.
* @param _assetHolder Address of allowed source for state channel funds
* @param _allowed True if asset holder is allowed
*/
function setAssetHolder(address _assetHolder, bool _allowed) external;

/**
* @notice Authorize or unauthorize an address to be an operator for the caller.
* @param _operator Address to authorize or unauthorize
9 changes: 0 additions & 9 deletions contracts/staking/IStakingExtension.sol
Original file line number Diff line number Diff line change
@@ -237,15 +237,6 @@ interface IStakingExtension is IStakingData {
*/
function rewardsDestination(address _indexer) external view returns (address);

/**
* @notice Getter for assetHolders[_maybeAssetHolder]:
* returns true if the address is an asset holder, i.e. an entity that can collect
* query fees into the Staking contract.
* @param _maybeAssetHolder The address that may or may not be an asset holder
* @return True if the address is an asset holder
*/
function assetHolders(address _maybeAssetHolder) external view returns (bool);

/**
* @notice Getter for subgraphAllocations[_subgraphDeploymentId]:
* returns the amount of tokens allocated to a subgraph deployment.
Loading