Skip to content

Commit

Permalink
Style update shared contracts (#11343)
Browse files Browse the repository at this point in the history 
* style update shared ocr2

* fix comment syntax in shared

* add struct packing comments and make natspec

---------

Co-authored-by: Austin Born <[email protected]>
  • Loading branch information
@RensR @austinborn
RensR and austinborn authored Nov 21, 2023
1 parent f7949c5 commit 5e3d68e
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 207 deletions.
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/v2_0/KeeperRegistry2_0.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ contract KeeperRegistry2_0 is
uint64 offchainConfigVersion,
bytes memory offchainConfig
) external override onlyOwner {
if (signers.length > maxNumOracles) revert TooManyOracles();
if (signers.length > MAX_NUM_ORACLES) revert TooManyOracles();
if (f == 0) revert IncorrectNumberOfFaultyOracles();
if (signers.length != transmitters.length || signers.length <= 3 * f) revert IncorrectNumberOfSigners();

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/v2_1/KeeperRegistry2_1.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, IE
uint64 offchainConfigVersion,
bytes memory offchainConfig
) public onlyOwner {
if (signers.length > maxNumOracles) revert TooManyOracles();
if (signers.length > MAX_NUM_ORACLES) revert TooManyOracles();
if (f == 0) revert IncorrectNumberOfFaultyOracles();
if (signers.length != transmitters.length || signers.length <= 3 * f) revert IncorrectNumberOfSigners();

Expand Down
6 changes: 2 additions & 4 deletions contracts/src/v0.8/shared/access/ConfirmedOwner.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ pragma solidity ^0.8.0;

import {ConfirmedOwnerWithProposal} from "./ConfirmedOwnerWithProposal.sol";

/**
* @title The ConfirmedOwner contract
* @notice A contract with helpers for basic contract ownership.
*/
/// @title The ConfirmedOwner contract
/// @notice A contract with helpers for basic contract ownership.
contract ConfirmedOwner is ConfirmedOwnerWithProposal {
constructor(address newOwner) ConfirmedOwnerWithProposal(newOwner, address(0)) {}
}
31 changes: 8 additions & 23 deletions contracts/src/v0.8/shared/access/ConfirmedOwnerWithProposal.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ pragma solidity ^0.8.0;

import {IOwnable} from "../interfaces/IOwnable.sol";

/**
* @title The ConfirmedOwner contract
* @notice A contract with helpers for basic contract ownership.
*/
/// @title The ConfirmedOwner contract
/// @notice A contract with helpers for basic contract ownership.
contract ConfirmedOwnerWithProposal is IOwnable {
address private s_owner;
address private s_pendingOwner;
Expand All @@ -24,17 +22,12 @@ contract ConfirmedOwnerWithProposal is IOwnable {
}
}

/**
* @notice Allows an owner to begin transferring ownership to a new address,
* pending.
*/
/// @notice Allows an owner to begin transferring ownership to a new address.
function transferOwnership(address to) public override onlyOwner {
_transferOwnership(to);
}

/**
* @notice Allows an ownership transfer to be completed by the recipient.
*/
/// @notice Allows an ownership transfer to be completed by the recipient.
function acceptOwnership() external override {
// solhint-disable-next-line custom-errors
require(msg.sender == s_pendingOwner, "Must be proposed owner");
Expand All @@ -46,16 +39,12 @@ contract ConfirmedOwnerWithProposal is IOwnable {
emit OwnershipTransferred(oldOwner, msg.sender);
}

/**
* @notice Get the current owner
*/
/// @notice Get the current owner
function owner() public view override returns (address) {
return s_owner;
}

/**
* @notice validate, transfer ownership, and emit relevant events
*/
/// @notice validate, transfer ownership, and emit relevant events
function _transferOwnership(address to) private {
// solhint-disable-next-line custom-errors
require(to != msg.sender, "Cannot transfer to self");
Expand All @@ -65,17 +54,13 @@ contract ConfirmedOwnerWithProposal is IOwnable {
emit OwnershipTransferRequested(s_owner, to);
}

/**
* @notice validate access
*/
/// @notice validate access
function _validateOwnership() internal view {
// solhint-disable-next-line custom-errors
require(msg.sender == s_owner, "Only callable by owner");
}

/**
* @notice Reverts if called by anyone other than the contract owner.
*/
/// @notice Reverts if called by anyone other than the contract owner.
modifier onlyOwner() {
_validateOwnership();
_;
Expand Down
26 changes: 11 additions & 15 deletions contracts/src/v0.8/shared/access/SimpleReadAccessController.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,18 @@ pragma solidity ^0.8.0;

import {SimpleWriteAccessController} from "./SimpleWriteAccessController.sol";

/**
* @title SimpleReadAccessController
* @notice Gives access to:
* - any externally owned account (note that off-chain actors can always read
* any contract storage regardless of on-chain access control measures, so this
* does not weaken the access control while improving usability)
* - accounts explicitly added to an access list
* @dev SimpleReadAccessController is not suitable for access controlling writes
* since it grants any externally owned account access! See
* SimpleWriteAccessController for that.
*/
/// @title SimpleReadAccessController
/// @notice Gives access to:
/// - any externally owned account (note that off-chain actors can always read
/// any contract storage regardless of on-chain access control measures, so this
/// does not weaken the access control while improving usability)
/// - accounts explicitly added to an access list
/// @dev SimpleReadAccessController is not suitable for access controlling writes
/// since it grants any externally owned account access! See
/// SimpleWriteAccessController for that.
contract SimpleReadAccessController is SimpleWriteAccessController {
/**
* @notice Returns the access of an address
* @param _user The address to query
*/
/// @notice Returns the access of an address
/// @param _user The address to query
function hasAccess(address _user, bytes memory _calldata) public view virtual override returns (bool) {
// solhint-disable-next-line avoid-tx-origin
return super.hasAccess(_user, _calldata) || _user == tx.origin;
Expand Down
40 changes: 12 additions & 28 deletions contracts/src/v0.8/shared/access/SimpleWriteAccessController.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@ pragma solidity ^0.8.0;
import {ConfirmedOwner} from "./ConfirmedOwner.sol";
import {AccessControllerInterface} from "../interfaces/AccessControllerInterface.sol";

/**
* @title SimpleWriteAccessController
* @notice Gives access to accounts explicitly added to an access list by the
* controller's owner.
* @dev does not make any special permissions for externally, see
* SimpleReadAccessController for that.
*/
/// @title SimpleWriteAccessController
/// @notice Gives access to accounts explicitly added to an access list by the controller's owner.
/// @dev does not make any special permissions for externally, see SimpleReadAccessController for that.
contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwner {
bool public checkEnabled;
mapping(address => bool) internal s_accessList;
Expand All @@ -24,18 +20,14 @@ contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwne
checkEnabled = true;
}

/**
* @notice Returns the access of an address
* @param _user The address to query
*/
/// @notice Returns the access of an address
/// @param _user The address to query
function hasAccess(address _user, bytes memory) public view virtual override returns (bool) {
return s_accessList[_user] || !checkEnabled;
}

/**
* @notice Adds an address to the access list
* @param _user The address to add
*/
/// @notice Adds an address to the access list
/// @param _user The address to add
function addAccess(address _user) external onlyOwner {
if (!s_accessList[_user]) {
s_accessList[_user] = true;
Expand All @@ -44,10 +36,8 @@ contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwne
}
}

/**
* @notice Removes an address from the access list
* @param _user The address to remove
*/
/// @notice Removes an address from the access list
/// @param _user The address to remove
function removeAccess(address _user) external onlyOwner {
if (s_accessList[_user]) {
s_accessList[_user] = false;
Expand All @@ -56,9 +46,7 @@ contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwne
}
}

/**
* @notice makes the access check enforced
*/
/// @notice makes the access check enforced
function enableAccessCheck() external onlyOwner {
if (!checkEnabled) {
checkEnabled = true;
Expand All @@ -67,9 +55,7 @@ contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwne
}
}

/**
* @notice makes the access check unenforced
*/
/// @notice makes the access check unenforced
function disableAccessCheck() external onlyOwner {
if (checkEnabled) {
checkEnabled = false;
Expand All @@ -78,9 +64,7 @@ contract SimpleWriteAccessController is AccessControllerInterface, ConfirmedOwne
}
}

/**
* @dev reverts if the caller does not have access
*/
/// @dev reverts if the caller does not have access
modifier checkAccess() {
// solhint-disable-next-line custom-errors
require(hasAccess(msg.sender, msg.data), "No access");
Expand Down
100 changes: 42 additions & 58 deletions contracts/src/v0.8/shared/ocr2/OCR2Abstract.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,20 @@ pragma solidity ^0.8.0;
import {ITypeAndVersion} from "../interfaces/ITypeAndVersion.sol";

abstract contract OCR2Abstract is ITypeAndVersion {
// Maximum number of oracles the offchain reporting protocol is designed for
// solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
uint256 internal constant maxNumOracles = 31;
// solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
uint256 private constant prefixMask = type(uint256).max << (256 - 16); // 0xFFFF00..00
// solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
uint256 private constant prefix = 0x0001 << (256 - 16); // 0x000100..00
uint256 internal constant MAX_NUM_ORACLES = 31;
uint256 private constant PREFIX_MASK = type(uint256).max << (256 - 16); // 0xFFFF00..00
uint256 private constant PREFIX = 0x0001 << (256 - 16); // 0x000100..00

/**
* @notice triggers a new run of the offchain reporting protocol
* @param previousConfigBlockNumber block in which the previous config was set, to simplify historic analysis
* @param configDigest configDigest of this configuration
* @param configCount ordinal number of this config setting among all config settings over the life of this contract
* @param signers ith element is address ith oracle uses to sign a report
* @param transmitters ith element is address ith oracle uses to transmit a report via the transmit method
* @param f maximum number of faulty/dishonest oracles the protocol can tolerate while still working correctly
* @param onchainConfig serialized configuration used by the contract (and possibly oracles)
* @param offchainConfigVersion version of the serialization format used for "offchainConfig" parameter
* @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract
*/
/// @notice triggers a new run of the offchain reporting protocol
/// @param previousConfigBlockNumber block in which the previous config was set, to simplify historic analysis
/// @param configDigest configDigest of this configuration
/// @param configCount ordinal number of this config setting among all config settings over the life of this contract
/// @param signers ith element is address ith oracle uses to sign a report
/// @param transmitters ith element is address ith oracle uses to transmit a report via the transmit method
/// @param f maximum number of faulty/dishonest oracles the protocol can tolerate while still working correctly
/// @param onchainConfig serialized configuration used by the contract (and possibly oracles)
/// @param offchainConfigVersion version of the serialization format used for "offchainConfig" parameter
/// @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract
event ConfigSet(
uint32 previousConfigBlockNumber,
bytes32 configDigest,
Expand All @@ -36,15 +30,13 @@ abstract contract OCR2Abstract is ITypeAndVersion {
bytes offchainConfig
);

/**
* @notice sets offchain reporting protocol configuration incl. participating oracles
* @param signers addresses with which oracles sign the reports
* @param transmitters addresses oracles use to transmit the reports
* @param f number of faulty oracles the system can tolerate
* @param onchainConfig serialized configuration used by the contract (and possibly oracles)
* @param offchainConfigVersion version number for offchainEncoding schema
* @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract
*/
/// @notice sets offchain reporting protocol configuration incl. participating oracles
/// @param signers addresses with which oracles sign the reports
/// @param transmitters addresses oracles use to transmit the reports
/// @param f number of faulty oracles the system can tolerate
/// @param onchainConfig serialized configuration used by the contract (and possibly oracles)
/// @param offchainConfigVersion version number for offchainEncoding schema
/// @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract
function setConfig(
address[] memory signers,
address[] memory transmitters,
Expand All @@ -54,12 +46,10 @@ abstract contract OCR2Abstract is ITypeAndVersion {
bytes memory offchainConfig
) external virtual;

/**
* @notice information about current offchain reporting protocol configuration
* @return configCount ordinal number of current config, out of all configs applied to this contract so far
* @return blockNumber block at which this config was set
* @return configDigest domain-separation tag for current config (see _configDigestFromConfigData)
*/
/// @notice information about current offchain reporting protocol configuration
/// @return configCount ordinal number of current config, out of all configs applied to this contract so far
/// @return blockNumber block at which this config was set
/// @return configDigest domain-separation tag for current config (see _configDigestFromConfigData)
function latestConfigDetails()
external
view
Expand Down Expand Up @@ -92,40 +82,34 @@ abstract contract OCR2Abstract is ITypeAndVersion {
)
)
);
return bytes32((prefix & prefixMask) | (h & ~prefixMask));
return bytes32((PREFIX & PREFIX_MASK) | (h & ~PREFIX_MASK));
}

/**
* @notice optionally emited to indicate the latest configDigest and epoch for
which a report was successfully transmited. Alternatively, the contract may
use latestConfigDigestAndEpoch with scanLogs set to false.
*/
/// @notice optionally emitted to indicate the latest configDigest and epoch for
/// which a report was successfully transmitted. Alternatively, the contract may
/// use latestConfigDigestAndEpoch with scanLogs set to false.
event Transmitted(bytes32 configDigest, uint32 epoch);

/**
* @notice optionally returns the latest configDigest and epoch for which a
report was successfully transmitted. Alternatively, the contract may return
scanLogs set to true and use Transmitted events to provide this information
to offchain watchers.
* @return scanLogs indicates whether to rely on the configDigest and epoch
returned or whether to scan logs for the Transmitted event instead.
* @return configDigest
* @return epoch
*/
/// @notice optionally returns the latest configDigest and epoch for which a
/// report was successfully transmitted. Alternatively, the contract may return
/// scanLogs set to true and use Transmitted events to provide this information
/// to offchain watchers.
/// @return scanLogs indicates whether to rely on the configDigest and epoch
/// returned or whether to scan logs for the Transmitted event instead.
/// @return configDigest
/// @return epoch
function latestConfigDigestAndEpoch()
external
view
virtual
returns (bool scanLogs, bytes32 configDigest, uint32 epoch);

/**
* @notice transmit is called to post a new report to the contract
* @param reportContext [0]: ConfigDigest, [1]: 27 byte padding, 4-byte epoch and 1-byte round, [2]: ExtraHash
* @param report serialized report, which the signatures are signing.
* @param rs ith element is the R components of the ith signature on report. Must have at most maxNumOracles entries
* @param ss ith element is the S components of the ith signature on report. Must have at most maxNumOracles entries
* @param rawVs ith element is the the V component of the ith signature
*/
/// @notice transmit is called to post a new report to the contract
/// @param reportContext [0]: ConfigDigest, [1]: 27 byte padding, 4-byte epoch and 1-byte round, [2]: ExtraHash
/// @param report serialized report, which the signatures are signing.
/// @param rs ith element is the R components of the ith signature on report. Must have at most MAX_NUM_ORACLES entries
/// @param ss ith element is the S components of the ith signature on report. Must have at most MAX_NUM_ORACLES entries
/// @param rawVs ith element is the the V component of the ith signature
function transmit(
// NOTE: If these parameters are changed, expectedMsgDataLength and/or
// TRANSMIT_MSGDATA_CONSTANT_LENGTH_COMPONENT need to be changed accordingly
Expand Down
Loading

0 comments on commit 5e3d68e

Please sign in to comment.