Skip to content

Commit c4f35e0

Browse files
abarmatabarmat
committed
staking: enforce funds collected from allowed asset holder
In a previous state channel implementation the AssetHolder was a multisig created for the purpose of funding each channel, as the multisig address was determined by the indexer-gateway relationship it needed to be passed in the allocate call and we needed to keep track of it in each allocation. In the newer implementation the AssetHolder will be shared (using ledger channels) across multiple channels, collateral is sent to the AssetHolder to keep an open tab to play multiple games, when a game finishes the adjudicator contract will end up unlocking funds from the AssetHolder and transferring it to the Staking contract according to the game played. The game played will carry the information about the destination address for collected funds, this is our allocationID. Implements: - Remove assetHolder parameter from all allocate() calls - Remove assetHolder state variable from Allocation - Add a function to set a new AssetHolder address as allowed - Verify in collect() function that the caller is any of the allowed AssetHolders
1 parent 2d4b867 commit c4f35e0

File tree

11 files changed

+103
-133
lines changed

11 files changed

+103
-133
lines changed

‎cli/commands/contracts/staking.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ export const allocate = async (cli: CLIEnvironment, cliArgs: CLIArgs): Promise<v
3434
const subgraphDeploymentID = cliArgs.subgraphDeploymentID
3535
const amount = parseGRT(cliArgs.amount)
3636
const allocationID = cliArgs.allocationID
37-
const assetHolder = cliArgs.assetHolder
3837
const metadata = cliArgs.metadata
3938
const staking = cli.contracts.Staking
4039

@@ -43,7 +42,7 @@ export const allocate = async (cli: CLIEnvironment, cliArgs: CLIArgs): Promise<v
4342
cli.wallet,
4443
staking,
4544
'allocate',
46-
...[subgraphDeploymentID, amount, allocationID, assetHolder, metadata],
45+
...[subgraphDeploymentID, amount, allocationID, metadata],
4746
)
4847
}
4948

@@ -197,12 +196,6 @@ export const stakingCommand = {
197196
requiresArg: true,
198197
demandOption: true,
199198
})
200-
.option('assetHolder', {
201-
description: 'Address of the contract that hold channel funds',
202-
type: 'string',
203-
requiresArg: true,
204-
demandOption: true,
205-
})
206199
.option('metadata', {
207200
description: 'IPFS hash of the metadata for the allocation',
208201
type: 'string',

‎contracts/staking/IStaking.sol

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ interface IStaking {
2727
uint256 closedAtEpoch; // Epoch when it was closed
2828
uint256 collectedFees; // Collected fees for the allocation
2929
uint256 effectiveAllocation; // Effective allocation when closed
30-
address assetHolder; // Authorized caller address of the collect() function
3130
uint256 accRewardsPerAllocatedToken; // Snapshot used for reward calc
3231
}
3332

@@ -83,6 +82,8 @@ interface IStaking {
8382

8483
function setSlasher(address _slasher, bool _allowed) external;
8584

85+
function setAssetHolder(address _assetHolder, bool _allowed) external;
86+
8687
// -- Operation --
8788

8889
function setOperator(address _operator, bool _allowed) external;
@@ -118,7 +119,6 @@ interface IStaking {
118119
bytes32 _subgraphDeploymentID,
119120
uint256 _tokens,
120121
address _allocationID,
121-
address _assetHolder,
122122
bytes32 _metadata
123123
) external;
124124

@@ -127,7 +127,6 @@ interface IStaking {
127127
bytes32 _subgraphDeploymentID,
128128
uint256 _tokens,
129129
address _allocationID,
130-
address _assetHolder,
131130
bytes32 _metadata
132131
) external;
133132

@@ -140,7 +139,6 @@ interface IStaking {
140139
bytes32 _subgraphDeploymentID,
141140
uint256 _tokens,
142141
address _allocationID,
143-
address _assetHolder,
144142
bytes32 _metadata
145143
) external;
146144

‎contracts/staking/Staking.sol

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
101101
uint256 epoch,
102102
uint256 tokens,
103103
address allocationID,
104-
bytes32 metadata,
105-
address assetHolder
104+
bytes32 metadata
106105
);
107106

108107
/**
@@ -156,9 +155,15 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
156155
);
157156

158157
/**
159-
* @dev Emitted when `caller` set `slasher` address as `enabled` to slash stakes.
158+
* @dev Emitted when `caller` set `slasher` address as `allowed` to slash stakes.
160159
*/
161-
event SlasherUpdate(address indexed caller, address indexed slasher, bool enabled);
160+
event SlasherUpdate(address indexed caller, address indexed slasher, bool allowed);
161+
162+
/**
163+
* @dev Emitted when `caller` set `assetHolder` address as `allowed` to send funds
164+
* to staking contract.
165+
*/
166+
event AssetHolderUpdate(address indexed caller, address indexed assetHolder, bool allowed);
162167

163168
/**
164169
* @dev Emitted when `indexer` set `operator` access.
@@ -363,10 +368,22 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
363368
* @param _allowed True if slasher is allowed
364369
*/
365370
function setSlasher(address _slasher, bool _allowed) external override onlyGovernor {
371+
require(_slasher != address(0), "!slasher");
366372
slashers[_slasher] = _allowed;
367373
emit SlasherUpdate(msg.sender, _slasher, _allowed);
368374
}
369375

376+
/**
377+
* @dev Set an address as allowed asset holder.
378+
* @param _assetHolder Address of allowed source for state channel funds
379+
* @param _allowed True if asset holder is allowed
380+
*/
381+
function setAssetHolder(address _assetHolder, bool _allowed) external override onlyGovernor {
382+
require(_assetHolder != address(0), "!assetHolder");
383+
assetHolders[_assetHolder] = _allowed;
384+
emit AssetHolderUpdate(msg.sender, _assetHolder, _allowed);
385+
}
386+
370387
/**
371388
* @dev Get the GRT token used by the contract.
372389
* @return GRT token contract address
@@ -680,24 +697,15 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
680697
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
681698
* @param _tokens Amount of tokens to allocate
682699
* @param _allocationID The allocation identifier
683-
* @param _assetHolder Authorized sender address of collected funds
684700
* @param _metadata IPFS hash for additional information about the allocation
685701
*/
686702
function allocate(
687703
bytes32 _subgraphDeploymentID,
688704
uint256 _tokens,
689705
address _allocationID,
690-
address _assetHolder,
691706
bytes32 _metadata
692707
) external override notPaused {
693-
_allocate(
694-
msg.sender,
695-
_subgraphDeploymentID,
696-
_tokens,
697-
_allocationID,
698-
_assetHolder,
699-
_metadata
700-
);
708+
_allocate(msg.sender, _subgraphDeploymentID, _tokens, _allocationID, _metadata);
701709
}
702710

703711
/**
@@ -706,18 +714,16 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
706714
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
707715
* @param _tokens Amount of tokens to allocate
708716
* @param _allocationID The allocation identifier
709-
* @param _assetHolder Authorized sender address of collected funds
710717
* @param _metadata IPFS hash for additional information about the allocation
711718
*/
712719
function allocateFrom(
713720
address _indexer,
714721
bytes32 _subgraphDeploymentID,
715722
uint256 _tokens,
716723
address _allocationID,
717-
address _assetHolder,
718724
bytes32 _metadata
719725
) external override notPaused {
720-
_allocate(_indexer, _subgraphDeploymentID, _tokens, _allocationID, _assetHolder, _metadata);
726+
_allocate(_indexer, _subgraphDeploymentID, _tokens, _allocationID, _metadata);
721727
}
722728

723729
/**
@@ -741,7 +747,6 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
741747
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
742748
* @param _tokens Amount of tokens to allocate
743749
* @param _allocationID The allocation identifier
744-
* @param _assetHolder Authorized sender address of collected funds
745750
* @param _metadata IPFS hash for additional information about the allocation
746751
*/
747752
function closeAndAllocate(
@@ -751,11 +756,10 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
751756
bytes32 _subgraphDeploymentID,
752757
uint256 _tokens,
753758
address _allocationID,
754-
address _assetHolder,
755759
bytes32 _metadata
756760
) external override notPaused {
757761
_closeAllocation(_closingAllocationID, _poi);
758-
_allocate(_indexer, _subgraphDeploymentID, _tokens, _allocationID, _assetHolder, _metadata);
762+
_allocate(_indexer, _subgraphDeploymentID, _tokens, _allocationID, _metadata);
759763
}
760764

761765
/**
@@ -767,13 +771,8 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
767771
// Allocation identifier validation
768772
require(_allocationID != address(0), "Invalid allocation");
769773

770-
// NOTE: commented out for easier test of state-channel integrations
771-
// NOTE: this validation might be removed in the future if no harm to the
772-
// NOTE: economic incentive structure is done by an external caller use
773-
// NOTE: of this function
774-
// The contract caller must be an asset holder registered during allocate()
775-
// Allocation memory alloc = allocations[_allocationID];
776-
// require(alloc.assetHolder == msg.sender, "caller is not authorized");
774+
// The contract caller must be an authorized asset holder
775+
require(assetHolders[msg.sender] == true, "!assetHolder");
777776

778777
// Transfer tokens to collect from the authorized sender
779778
require(graphToken().transferFrom(msg.sender, address(this), _tokens), "!transfer");
@@ -816,7 +815,6 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
816815
alloc.closedAtEpoch = 0;
817816
alloc.collectedFees = 0;
818817
alloc.effectiveAllocation = 0;
819-
alloc.assetHolder = address(0); // This avoid collect() to be called
820818

821819
// -- Effects --
822820

@@ -877,15 +875,13 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
877875
* @param _subgraphDeploymentID ID of the SubgraphDeployment where tokens will be allocated
878876
* @param _tokens Amount of tokens to allocate
879877
* @param _allocationID The allocationID will work to identify collected funds related to this allocation
880-
* @param _assetHolder Authorized sender address of collected funds
881878
* @param _metadata Metadata related to the allocation
882879
*/
883880
function _allocate(
884881
address _indexer,
885882
bytes32 _subgraphDeploymentID,
886883
uint256 _tokens,
887884
address _allocationID,
888-
address _assetHolder,
889885
bytes32 _metadata
890886
) internal {
891887
require(_onlyAuth(_indexer), "!auth");
@@ -918,7 +914,6 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
918914
0, // closedAtEpoch
919915
0, // Initialize collected fees
920916
0, // Initialize effective allocation
921-
_assetHolder, // Source address for allocation collected funds
922917
_updateRewards(_subgraphDeploymentID) // Initialize accumulated rewards per stake allocated
923918
);
924919
allocations[_allocationID] = alloc;
@@ -938,8 +933,7 @@ contract Staking is StakingV1Storage, GraphUpgradeable, IStaking {
938933
alloc.createdAtEpoch,
939934
alloc.tokens,
940935
_allocationID,
941-
_metadata,
942-
_assetHolder
936+
_metadata
943937
);
944938
}
945939

‎contracts/staking/StakingStorage.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,9 @@ contract StakingV1Storage is Managed {
6767

6868
// Operator auth : indexer => operator
6969
mapping(address => mapping(address => bool)) public operatorAuth;
70+
71+
// -- Asset Holders --
72+
73+
// Allowed AssetHolders: assetHolder => is allowed
74+
mapping(address => bool) public assetHolders;
7075
}

‎test/disputes/poi.test.ts

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,11 @@ import {
2020
const { keccak256 } = utils
2121

2222
describe('DisputeManager:POI', async () => {
23-
let me: Account
2423
let other: Account
2524
let governor: Account
2625
let arbitrator: Account
2726
let indexer: Account
2827
let fisherman: Account
29-
let indexer2: Account
3028
let assetHolder: Account
3129

3230
let fixture: NetworkFixture
@@ -67,27 +65,12 @@ describe('DisputeManager:POI', async () => {
6765
await staking.connect(indexerWallet.signer).stake(indexerTokens)
6866
await staking
6967
.connect(indexerWallet.signer)
70-
.allocate(
71-
subgraphDeploymentID,
72-
indexerAllocatedTokens,
73-
indexerAllocationID,
74-
assetHolder.address,
75-
metadata,
76-
)
68+
.allocate(subgraphDeploymentID, indexerAllocatedTokens, indexerAllocationID, metadata)
7769
}
7870
}
7971

8072
before(async function () {
81-
;[
82-
me,
83-
other,
84-
governor,
85-
arbitrator,
86-
indexer,
87-
fisherman,
88-
indexer2,
89-
assetHolder,
90-
] = await getAccounts()
73+
;[other, governor, arbitrator, indexer, fisherman, assetHolder] = await getAccounts()
9174

9275
fixture = new NetworkFixture()
9376
;({ disputeManager, epochManager, grt, staking } = await fixture.load(
@@ -99,6 +82,9 @@ describe('DisputeManager:POI', async () => {
9982
// Give some funds to the fisherman
10083
await grt.connect(governor.signer).mint(fisherman.address, fishermanTokens)
10184
await grt.connect(fisherman.signer).approve(disputeManager.address, fishermanTokens)
85+
86+
// Allow the asset holder
87+
await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true)
10288
})
10389

10490
beforeEach(async function () {
@@ -141,13 +127,7 @@ describe('DisputeManager:POI', async () => {
141127
await staking.connect(indexer.signer).stake(indexerTokens)
142128
const tx1 = await staking
143129
.connect(indexer.signer)
144-
.allocate(
145-
subgraphDeploymentID,
146-
indexerAllocatedTokens,
147-
allocationID,
148-
assetHolder.address,
149-
metadata,
150-
)
130+
.allocate(subgraphDeploymentID, indexerAllocatedTokens, allocationID, metadata)
151131
const receipt1 = await tx1.wait()
152132
const event1 = staking.interface.parseLog(receipt1.logs[0]).args
153133
await advanceToNextEpoch(epochManager) // wait the required one epoch to close allocation

‎test/disputes/query.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ describe('DisputeManager:Query', async () => {
133133
dispute.receipt.subgraphDeploymentID,
134134
indexerAllocatedTokens,
135135
indexerAllocationID,
136-
assetHolder.address,
137136
metadata,
138137
)
139138
}
@@ -162,6 +161,9 @@ describe('DisputeManager:Query', async () => {
162161
await grt.connect(governor.signer).mint(fisherman.address, fishermanTokens)
163162
await grt.connect(fisherman.signer).approve(disputeManager.address, fishermanTokens)
164163

164+
// Allow the asset holder
165+
await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true)
166+
165167
// Create an attestation
166168
const attestation = await buildAttestation(receipt, indexer1ChannelKey.privKey)
167169

@@ -217,7 +219,6 @@ describe('DisputeManager:Query', async () => {
217219
dispute.receipt.subgraphDeploymentID,
218220
indexerAllocatedTokens,
219221
indexer1ChannelKey.address,
220-
assetHolder.address,
221222
metadata,
222223
)
223224
const receipt1 = await tx1.wait()

0 commit comments

Comments
 (0)