Skip to content

Commit

Permalink
Merge pull request #3137 from keep-network/beacon-dkg-extra-gas
Browse files Browse the repository at this point in the history 
Added extra gas requirement for Beacon DKG result challenge
  • Loading branch information
@tomaszslabon
tomaszslabon authored Aug 8, 2022
2 parents b699023 + 0973e85 commit bf41678
Show file tree
Hide file tree
Showing 10 changed files with 320 additions and 27 deletions.
8 changes: 7 additions & 1 deletion solidity/ecdsa/contracts/WalletRegistry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,12 @@ contract WalletRegistry is
/// invalid it reverts the DKG back to the result submission phase.
/// @param dkgResult Result to challenge. Must match the submitted result
/// stored during `submitDkgResult`.
/// @dev Due to EIP-150 1/64 of the gas is not forwarded to the call, and
/// will be kept to execute the remaining operations in the function
/// after the call inside the try-catch. To eliminate a class of
/// attacks related to the gas limit manipulation, this function
/// requires an extra amount of gas to be left at the end of the
/// execution.
function challengeDkgResult(DKG.Result calldata dkgResult) external {
(
bytes32 maliciousDkgResultHash,
Expand Down Expand Up @@ -829,7 +835,7 @@ contract WalletRegistry is
);
}

// Due to EIP150, 1/64 of the gas is not forwarded to the call, and
// Due to EIP-150, 1/64 of the gas is not forwarded to the call, and
// will be kept to execute the remaining operations in the function
// after the call inside the try-catch.
//
Expand Down
4 changes: 2 additions & 2 deletions solidity/ecdsa/contracts/libraries/EcdsaDkg.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ library EcdsaDkg {
/// and the rest of the function is executed with the remaining
/// 1/64 of gas, we require an extra gas amount to be left at the
/// end of the call to the function challenging DKG result and
/// wrapping the call to EcdsaDkgValidator in TokenStaking contract
/// inside a try-catch.
/// wrapping the call to EcdsaDkgValidator and TokenStaking
/// contracts inside a try-catch.
function requireChallengeExtraGas(Data storage self) internal view {
require(
gasleft() >= self.parameters.resultChallengeExtraGas,
Expand Down
37 changes: 31 additions & 6 deletions solidity/random-beacon/contracts/RandomBeacon.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {

/// @notice Seed value used for the genesis group selection.
/// https://www.wolframalpha.com/input/?i=pi+to+78+digits
uint256 public constant genesisSeed =
uint256 internal constant genesisSeed =
31415926535897932384626433832795028841971693993751058209749445923078164062862;

// Governable parameters
Expand Down Expand Up @@ -178,6 +178,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgResultSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -429,6 +430,8 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
// DKG result challenge period length is set to 48h, assuming
// 15s block time.
//
// The extra gas required for DKG result challenge is 50k units.
//
// DKG result submission timeout, gives each member 20 blocks to submit
// the result. Assuming 15s block time, it is ~8h to submit the result
// in the pessimistic case.
Expand All @@ -438,7 +441,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
//
// With these parameters, the happy path takes no more than 56 hours.
// In practice, it should take about 48 hours (just the challenge time).
dkg.setParameters(11_520, 1_280, 20);
dkg.setParameters(11_520, 50_000, 1_280, 20);

// Relay entry soft timeot gives each of 64 members 20 blocks to submit
// the result.
Expand Down Expand Up @@ -541,20 +544,23 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
/// @param groupLifetime New group lifetime in blocks
/// @param dkgResultChallengePeriodLength New DKG result challenge period
/// length
/// @param dkgResultChallengeExtraGas New DKG result challenge extra gas
/// @param dkgResultSubmissionTimeout New DKG result submission timeout
/// @param dkgSubmitterPrecedencePeriodLength New DKG result submitter
/// precedence period length
function updateGroupCreationParameters(
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
) external onlyGovernance {
_groupCreationFrequency = groupCreationFrequency;
groups.setGroupLifetime(groupLifetime);
dkg.setParameters(
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
Expand All @@ -563,6 +569,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
groupCreationFrequency,
groupLifetime,
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -904,6 +911,12 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
/// the DKG result submission.
/// @param dkgResult Result to challenge. Must match the submitted result
/// stored during `submitDkgResult`.
/// @dev Due to EIP-150 1/64 of the gas is not forwarded to the call, and
/// will be kept to execute the remaining operations in the function
/// after the call inside the try-catch. To eliminate a class of
/// attacks related to the gas limit manipulation, this function
/// requires an extra amount of gas to be left at the end of the
/// execution.
function challengeDkgResult(DKG.Result calldata dkgResult) external {
(bytes32 maliciousResultHash, uint32 maliciousSubmitter) = dkg
.challengeResult(dkgResult);
Expand Down Expand Up @@ -941,6 +954,17 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
maliciousSubmitterAddresses
);
}

// Due to EIP-150, 1/64 of the gas is not forwarded to the call, and
// will be kept to execute the remaining operations in the function
// after the call inside the try-catch.
//
// To ensure there is no way for the caller to manipulate gas limit in
// such a way that the call inside try-catch fails with out-of-gas and
// the rest of the function is executed with the remaining 1/64 of gas,
// we require an extra gas amount to be left at the end of the call to
// `challengeDkgResult`.
dkg.requireChallengeExtraGas();
}

/// @notice Check current group creation state.
Expand Down Expand Up @@ -1270,10 +1294,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {

require(nonce == inactivityClaimNonce[groupId], "Invalid nonce");

require(
groups.isGroupActive(groupId),
"Group must be active and non-terminated"
);
require(groups.isGroupActive(groupId), "Group is not active");

Groups.Group storage group = groups.getGroup(groupId);

Expand Down Expand Up @@ -1494,6 +1515,8 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
/// accepted and the group registered in the pool of active groups.
/// If the challenge gets accepted, all operators who signed the
/// malicious result get slashed for and the notifier gets rewarded.
/// @return dkgResultChallengeExtraGas The extra gas required to be left at
/// the end of the challenge DKG result transaction.
/// @return dkgResultSubmissionTimeout Timeout in blocks for a group to
/// submit the DKG result. All members are eligible to submit the
/// DKG result. If `dkgResultSubmissionTimeout` passes without the
Expand All @@ -1510,6 +1533,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
)
Expand All @@ -1518,6 +1542,7 @@ contract RandomBeacon is IRandomBeacon, IApplication, Governable, Reimbursable {
_groupCreationFrequency,
groups.groupLifetime,
dkg.parameters.resultChallengePeriodLength,
dkg.parameters.resultChallengeExtraGas,
dkg.parameters.resultSubmissionTimeout,
dkg.parameters.submitterPrecedencePeriodLength
);
Expand Down
77 changes: 77 additions & 0 deletions solidity/random-beacon/contracts/RandomBeaconGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ contract RandomBeaconGovernance is Ownable {
uint256 public newDkgResultChallengePeriodLength;
uint256 public dkgResultChallengePeriodLengthChangeInitiated;

uint256 public newDkgResultChallengeExtraGas;
uint256 public dkgResultChallengeExtraGasChangeInitiated;

uint256 public newDkgResultSubmissionTimeout;
uint256 public dkgResultSubmissionTimeoutChangeInitiated;

Expand Down Expand Up @@ -147,6 +150,12 @@ contract RandomBeaconGovernance is Ownable {
uint256 dkgResultChallengePeriodLength
);

event DkgResultChallengeExtraGasUpdateStarted(
uint256 dkgResultChallengeExtraGas,
uint256 timestamp
);
event DkgResultChallengeExtraGasUpdated(uint256 dkgResultChallengeExtraGas);

event DkgResultSubmissionTimeoutUpdateStarted(
uint256 dkgResultSubmissionTimeout,
uint256 timestamp
Expand Down Expand Up @@ -515,6 +524,7 @@ contract RandomBeaconGovernance is Ownable {
,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
) = randomBeacon.groupCreationParameters();
Expand All @@ -523,6 +533,7 @@ contract RandomBeaconGovernance is Ownable {
newGroupCreationFrequency,
groupLifetime,
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -562,6 +573,7 @@ contract RandomBeaconGovernance is Ownable {
uint256 groupCreationFrequency,
,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
) = randomBeacon.groupCreationParameters();
Expand All @@ -570,6 +582,7 @@ contract RandomBeaconGovernance is Ownable {
groupCreationFrequency,
newGroupLifetime,
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -613,6 +626,7 @@ contract RandomBeaconGovernance is Ownable {
uint256 groupCreationFrequency,
uint256 groupLifetime,
,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
) = randomBeacon.groupCreationParameters();
Expand All @@ -621,13 +635,60 @@ contract RandomBeaconGovernance is Ownable {
groupCreationFrequency,
groupLifetime,
newDkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
dkgResultChallengePeriodLengthChangeInitiated = 0;
newDkgResultChallengePeriodLength = 0;
}

/// @notice Begins the DKG result challenge extra gas update process.
/// @dev Can be called only by the contract owner.
/// @param _newDkgResultChallengeExtraGas New DKG result challenge extra gas
function beginDkgResultChallengeExtraGasUpdate(
uint256 _newDkgResultChallengeExtraGas
) external onlyOwner {
/* solhint-disable not-rely-on-time */
newDkgResultChallengeExtraGas = _newDkgResultChallengeExtraGas;
dkgResultChallengeExtraGasChangeInitiated = block.timestamp;
emit DkgResultChallengeExtraGasUpdateStarted(
_newDkgResultChallengeExtraGas,
block.timestamp
);
/* solhint-enable not-rely-on-time */
}

/// @notice Finalizes the DKG result challenge extra gas update process.
/// @dev Can be called only by the contract owner, after the governance
/// delay elapses.
function finalizeDkgResultChallengeExtraGasUpdate()
external
onlyOwner
onlyAfterGovernanceDelay(dkgResultChallengeExtraGasChangeInitiated)
{
emit DkgResultChallengeExtraGasUpdated(newDkgResultChallengeExtraGas);
(
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
,
uint256 dkgResultSubmissionTimeout,
uint256 dkgSubmitterPrecedencePeriodLength
) = randomBeacon.groupCreationParameters();
// slither-disable-next-line reentrancy-no-eth
randomBeacon.updateGroupCreationParameters(
groupCreationFrequency,
groupLifetime,
dkgResultChallengePeriodLength,
newDkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
dkgResultChallengeExtraGasChangeInitiated = 0;
newDkgResultChallengeExtraGas = 0;
}

/// @notice Begins the DKG result submission timeout update
/// process.
/// @dev Can be called only by the contract owner.
Expand Down Expand Up @@ -664,6 +725,7 @@ contract RandomBeaconGovernance is Ownable {
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
,
uint256 dkgSubmitterPrecedencePeriodLength
) = randomBeacon.groupCreationParameters();
Expand All @@ -672,6 +734,7 @@ contract RandomBeaconGovernance is Ownable {
groupCreationFrequency,
groupLifetime,
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
newDkgResultSubmissionTimeout,
dkgSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -717,6 +780,7 @@ contract RandomBeaconGovernance is Ownable {
uint256 groupCreationFrequency,
uint256 groupLifetime,
uint256 dkgResultChallengePeriodLength,
uint256 dkgResultChallengeExtraGas,
uint256 dkgResultSubmissionTimeout,

) = randomBeacon.groupCreationParameters();
Expand All @@ -725,6 +789,7 @@ contract RandomBeaconGovernance is Ownable {
groupCreationFrequency,
groupLifetime,
dkgResultChallengePeriodLength,
dkgResultChallengeExtraGas,
dkgResultSubmissionTimeout,
newDkgSubmitterPrecedencePeriodLength
);
Expand Down Expand Up @@ -1490,6 +1555,18 @@ contract RandomBeaconGovernance is Ownable {
);
}

/// @notice Get the time remaining until the DKG result challenge extra
/// gas can be updated.
/// @return Remaining time in seconds.
function getRemainingDkgResultChallengeExtraGasUpdateTime()
external
view
returns (uint256)
{
return
getRemainingChangeTime(dkgResultChallengeExtraGasChangeInitiated);
}

/// @notice Get the time remaining until the DKG result submission timeout
/// can be updated.
/// @return Remaining time in seconds.
Expand Down
Loading

0 comments on commit bf41678

Please sign in to comment.