Skip to content

Commit 9f56d4b

Browse files
authored
Merge pull request #1 from Aerilym/feat/sn-contrib-update-events
Fix multi-contrib rescuing of non-finalized funds and update state update events
2 parents e078ea4 + 96be7d9 commit 9f56d4b

File tree

7 files changed

+272
-84
lines changed

7 files changed

+272
-84
lines changed

.openzeppelin/unknown-421614.json

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9306,7 +9306,7 @@
93069306
"label": "stakingRewardsContract",
93079307
"offset": 0,
93089308
"slot": "0",
9309-
"type": "t_contract(IServiceNodeRewards)8197",
9309+
"type": "t_contract(IServiceNodeRewards)10632",
93109310
"contract": "ServiceNodeContributionFactory",
93119311
"src": "contracts/ServiceNodeContributionFactory.sol:11"
93129312
},
@@ -9386,7 +9386,158 @@
93869386
"label": "uint64",
93879387
"numberOfBytes": "8"
93889388
},
9389-
"t_contract(IServiceNodeRewards)8197": {
9389+
"t_contract(IServiceNodeRewards)10632": {
9390+
"label": "contract IServiceNodeRewards",
9391+
"numberOfBytes": "20"
9392+
},
9393+
"t_mapping(t_address,t_bool)": {
9394+
"label": "mapping(address => bool)",
9395+
"numberOfBytes": "32"
9396+
}
9397+
},
9398+
"namespaces": {
9399+
"erc7201:openzeppelin.storage.Pausable": [
9400+
{
9401+
"contract": "PausableUpgradeable",
9402+
"label": "_paused",
9403+
"type": "t_bool",
9404+
"src": "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol:21",
9405+
"offset": 0,
9406+
"slot": "0"
9407+
}
9408+
],
9409+
"erc7201:openzeppelin.storage.Ownable2Step": [
9410+
{
9411+
"contract": "Ownable2StepUpgradeable",
9412+
"label": "_pendingOwner",
9413+
"type": "t_address",
9414+
"src": "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol:23",
9415+
"offset": 0,
9416+
"slot": "0"
9417+
}
9418+
],
9419+
"erc7201:openzeppelin.storage.Ownable": [
9420+
{
9421+
"contract": "OwnableUpgradeable",
9422+
"label": "_owner",
9423+
"type": "t_address",
9424+
"src": "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol:24",
9425+
"offset": 0,
9426+
"slot": "0"
9427+
}
9428+
],
9429+
"erc7201:openzeppelin.storage.Initializable": [
9430+
{
9431+
"contract": "Initializable",
9432+
"label": "_initialized",
9433+
"type": "t_uint64",
9434+
"src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:69",
9435+
"offset": 0,
9436+
"slot": "0"
9437+
},
9438+
{
9439+
"contract": "Initializable",
9440+
"label": "_initializing",
9441+
"type": "t_bool",
9442+
"src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:73",
9443+
"offset": 8,
9444+
"slot": "0"
9445+
}
9446+
]
9447+
}
9448+
}
9449+
},
9450+
"72f60db4a84bdc028c0dc99f8e6688a7c85ddfd53874b06003a8bc7b2044a1a1": {
9451+
"address": "0xe7EbB8227aebE1E0049aB00AeD23f428a1256130",
9452+
"txHash": "0x0f80fd169274904025486bc4506b7f4e8cbd3ff597e8e46e39fdf36787224945",
9453+
"layout": {
9454+
"solcVersion": "0.8.26",
9455+
"storage": [
9456+
{
9457+
"label": "stakingRewardsContract",
9458+
"offset": 0,
9459+
"slot": "0",
9460+
"type": "t_contract(IServiceNodeRewards)7940",
9461+
"contract": "ServiceNodeContributionFactory",
9462+
"src": "contracts/ServiceNodeContributionFactory.sol:11"
9463+
},
9464+
{
9465+
"label": "deployedContracts",
9466+
"offset": 0,
9467+
"slot": "1",
9468+
"type": "t_mapping(t_address,t_bool)",
9469+
"contract": "ServiceNodeContributionFactory",
9470+
"src": "contracts/ServiceNodeContributionFactory.sol:15"
9471+
}
9472+
],
9473+
"types": {
9474+
"t_address": {
9475+
"label": "address",
9476+
"numberOfBytes": "20"
9477+
},
9478+
"t_bool": {
9479+
"label": "bool",
9480+
"numberOfBytes": "1"
9481+
},
9482+
"t_struct(InitializableStorage)141_storage": {
9483+
"label": "struct Initializable.InitializableStorage",
9484+
"members": [
9485+
{
9486+
"label": "_initialized",
9487+
"type": "t_uint64",
9488+
"offset": 0,
9489+
"slot": "0"
9490+
},
9491+
{
9492+
"label": "_initializing",
9493+
"type": "t_bool",
9494+
"offset": 8,
9495+
"slot": "0"
9496+
}
9497+
],
9498+
"numberOfBytes": "32"
9499+
},
9500+
"t_struct(Ownable2StepStorage)13_storage": {
9501+
"label": "struct Ownable2StepUpgradeable.Ownable2StepStorage",
9502+
"members": [
9503+
{
9504+
"label": "_pendingOwner",
9505+
"type": "t_address",
9506+
"offset": 0,
9507+
"slot": "0"
9508+
}
9509+
],
9510+
"numberOfBytes": "32"
9511+
},
9512+
"t_struct(OwnableStorage)74_storage": {
9513+
"label": "struct OwnableUpgradeable.OwnableStorage",
9514+
"members": [
9515+
{
9516+
"label": "_owner",
9517+
"type": "t_address",
9518+
"offset": 0,
9519+
"slot": "0"
9520+
}
9521+
],
9522+
"numberOfBytes": "32"
9523+
},
9524+
"t_struct(PausableStorage)233_storage": {
9525+
"label": "struct PausableUpgradeable.PausableStorage",
9526+
"members": [
9527+
{
9528+
"label": "_paused",
9529+
"type": "t_bool",
9530+
"offset": 0,
9531+
"slot": "0"
9532+
}
9533+
],
9534+
"numberOfBytes": "32"
9535+
},
9536+
"t_uint64": {
9537+
"label": "uint64",
9538+
"numberOfBytes": "8"
9539+
},
9540+
"t_contract(IServiceNodeRewards)7940": {
93909541
"label": "contract IServiceNodeRewards",
93919542
"numberOfBytes": "20"
93929543
},

contracts/ServiceNodeContribution.sol

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,19 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
6060
// a public contributor fulfills the node).
6161
bool public manualFinalize;
6262

63-
// Modifers
63+
// Modifiers
6464
modifier onlyOperator() {
6565
if (msg.sender != operator)
6666
revert OnlyOperatorIsAuthorised(msg.sender, operator);
6767
_;
6868
}
6969

70+
modifier requireWaitForOperatorContribStatus() {
71+
if (status != Status.WaitForOperatorContrib)
72+
revert RequireWaitForOperatorContribStatus(status);
73+
_;
74+
}
75+
7076
/// @notice Constructs a multi-contribution node contract for the
7177
/// specified `_stakingRewardsContract`.
7278
///
@@ -115,16 +121,17 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
115121

116122
function _updateManualFinalize(bool value) private {
117123
manualFinalize = value;
124+
emit UpdateManualFinalize(value);
118125
}
119126

120127
function updateFee(uint16 fee) external onlyOperator { _updateFee(fee); }
121128

122-
function _updateFee(uint16 fee) private {
123-
if (status != Status.WaitForOperatorContrib)
124-
revert FeeUpdateNotPossible(status);
129+
function _updateFee(uint16 fee) private requireWaitForOperatorContribStatus {
125130
if (fee > MAX_FEE)
126131
revert FeeExceedsPossibleValue(fee, MAX_FEE);
127132
_serviceNodeParams.fee = fee;
133+
134+
emit UpdateFee(fee);
128135
}
129136

130137
function updatePubkeys(BN256G1.G1Point memory newBLSPubkey,
@@ -139,10 +146,7 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
139146
IServiceNodeRewards.BLSSignatureParams memory newBLSSig,
140147
uint256 ed25519Pubkey,
141148
uint256 ed25519Sig0,
142-
uint256 ed25519Sig1) private {
143-
if (status != Status.WaitForOperatorContrib)
144-
revert PubkeyUpdateNotPossible(status);
145-
149+
uint256 ed25519Sig1) private requireWaitForOperatorContribStatus {
146150
stakingRewardsContract.validateProofOfPossession(newBLSPubkey, newBLSSig, operator, ed25519Pubkey);
147151

148152
// NOTE: Update BLS keys
@@ -153,16 +157,15 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
153157
_serviceNodeParams.serviceNodePubkey = ed25519Pubkey;
154158
_serviceNodeParams.serviceNodeSignature1 = ed25519Sig0;
155159
_serviceNodeParams.serviceNodeSignature2 = ed25519Sig1;
160+
161+
emit UpdatePubkeys(newBLSPubkey, ed25519Pubkey);
156162
}
157163

158164
function updateReservedContributors(IServiceNodeRewards.ReservedContributor[] memory reserved) external onlyOperator {
159165
_updateReservedContributors(reserved);
160166
}
161167

162-
function _updateReservedContributors(IServiceNodeRewards.ReservedContributor[] memory reserved) private {
163-
if (status != Status.WaitForOperatorContrib)
164-
revert ReservedContributorUpdateNotPossible(status);
165-
168+
function _updateReservedContributors(IServiceNodeRewards.ReservedContributor[] memory reserved) private requireWaitForOperatorContribStatus {
166169
// NOTE: Remove old reserved contributions
167170
{
168171
uint256 length = reservedContributionsAddresses.length;
@@ -213,6 +216,8 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
213216

214217
unchecked { i += 1; }
215218
}
219+
220+
emit UpdateReservedContributors(reserved);
216221
}
217222

218223
// @notice Select the beneficiary as the `beneficiary` is not the
@@ -249,7 +254,7 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
249254
if (!updated)
250255
revert NonContributorUpdatedBeneficiary(stakerAddr);
251256

252-
emit UpdateStakerBeneficiary(stakerAddr, oldBeneficiary, desiredBeneficiary);
257+
emit UpdateStakerBeneficiary(stakerAddr, desiredBeneficiary);
253258
}
254259

255260
function contributeFunds(uint256 amount, address beneficiary) external { _contributeFunds(msg.sender, beneficiary, amount); }
@@ -271,7 +276,7 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
271276
if (caller != operator)
272277
revert FirstContributionMustBeOperator(caller, operator);
273278
status = Status.OpenForPublicContrib;
274-
emit OpenForPublicContribution(_serviceNodeParams.serviceNodePubkey, operator, _serviceNodeParams.fee);
279+
emit OpenForPublicContribution();
275280
}
276281

277282
// NOTE: Verify the contribution
@@ -318,8 +323,8 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
318323
// State transition before calling out to external code to mitigate
319324
// re-entrancy.
320325
if (currTotalContribution == stakingRequirement) {
321-
emit Filled(_serviceNodeParams.serviceNodePubkey, operator);
322326
status = Status.WaitForFinalized;
327+
emit Filled();
323328
}
324329

325330
// NOTE: Transfer funds from sender to contract
@@ -340,7 +345,7 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
340345

341346
// NOTE: Finalize the contract
342347
status = Status.Finalized;
343-
emit Finalized(_serviceNodeParams.serviceNodePubkey);
348+
emit Finalized();
344349

345350
uint256 length = _contributorAddresses.length;
346351
IServiceNodeRewards.Contributor[] memory contributors = new IServiceNodeRewards.Contributor[](length);
@@ -382,6 +387,8 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
382387
IServiceNodeRewards.ReservedContributor[] memory zero = new IServiceNodeRewards.ReservedContributor[](0);
383388
_updateReservedContributors(zero);
384389
}
390+
391+
emit Reset();
385392
}
386393

387394
function resetUpdateAndContribute(BN256G1.G1Point memory key,
@@ -410,19 +417,6 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
410417
_contributeFunds(operator, beneficiary, amount);
411418
}
412419

413-
function resetUpdateFeeReservedAndContribute(uint16 fee,
414-
IServiceNodeRewards.ReservedContributor[] memory reserved,
415-
bool _manualFinalize,
416-
address beneficiary,
417-
uint256 amount) external onlyOperator {
418-
_reset();
419-
_updateFee(fee);
420-
_updateReservedContributors(reserved);
421-
_updateManualFinalize(_manualFinalize);
422-
if (amount > 0)
423-
_contributeFunds(operator, beneficiary, amount);
424-
}
425-
426420
function rescueERC20(address tokenAddress) external onlyOperator {
427421
// NOTE: ERC20 tokens sent to the contract can only be rescued after the
428422
// contract is finalized or the contract has been reset
@@ -437,15 +431,16 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
437431
// This allows them to refund any other tokens that might have
438432
// mistakenly been sent throughout the lifetime of the contract without
439433
// giving them access to contributor tokens.
440-
if (status != Status.Finalized && status == Status.WaitForOperatorContrib)
434+
if (status == Status.Finalized || status == Status.WaitForOperatorContrib) {
435+
IERC20 token = IERC20(tokenAddress);
436+
uint256 balance = token.balanceOf(address(this));
437+
if (balance <= 0)
438+
revert RescueBalanceIsEmpty(tokenAddress);
439+
token.safeTransfer(operator, balance);
440+
} else {
441441
revert RescueNotPossible(status);
442+
}
442443

443-
IERC20 token = IERC20(tokenAddress);
444-
uint256 balance = token.balanceOf(address(this));
445-
if (balance <= 0)
446-
revert RescueBalanceIsEmpty(tokenAddress);
447-
448-
token.safeTransfer(operator, balance);
449444
}
450445

451446
function withdrawContribution() external {

contracts/ServiceNodeContributionFactory.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ contract ServiceNodeContributionFactory is Initializable, Ownable2StepUpgradeabl
1515
mapping(address => bool) public deployedContracts;
1616

1717
// Events
18-
event NewServiceNodeContributionContract(address indexed contributorContract, uint256 serviceNodePubkey);
18+
event NewServiceNodeContributionContract(address indexed contributorContract, uint256 serviceNodePubkey, address operator);
1919

2020
function initialize(address _stakingRewardsContract) public initializer {
2121
stakingRewardsContract = IServiceNodeRewards(_stakingRewardsContract);
@@ -43,7 +43,7 @@ contract ServiceNodeContributionFactory is Initializable, Ownable2StepUpgradeabl
4343

4444
result = address(newContract);
4545
deployedContracts[result] = true;
46-
emit NewServiceNodeContributionContract(result, params.serviceNodePubkey);
46+
emit NewServiceNodeContributionContract(result, params.serviceNodePubkey, tx.origin);
4747
return result;
4848
}
4949

0 commit comments

Comments
 (0)