Skip to content

Commit

Permalink
chore(protoocl): optimize code based on OZ defender suggestions (#18879)
Browse files Browse the repository at this point in the history
  • Loading branch information
dantaik authored Feb 6, 2025
1 parent 4a9202f commit 760fb56
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract {
external
onlyOwner
{
for (uint256 i; i < serialNumBatch.length; ++i) {
uint256 size = serialNumBatch.length;
for (uint256 i; i < size; ++i) {
if (serialNumIsRevoked[index][serialNumBatch[i]]) {
continue;
}
Expand All @@ -108,7 +109,8 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract {
external
onlyOwner
{
for (uint256 i; i < serialNumBatch.length; ++i) {
uint256 size = serialNumBatch.length;
for (uint256 i; i < size; ++i) {
if (!serialNumIsRevoked[index][serialNumBatch[i]]) {
continue;
}
Expand Down Expand Up @@ -208,7 +210,8 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract {
bool isvprodidMatched = quoteEnclaveReport.isvProdId == enclaveId.isvprodid;

bool tcbFound;
for (uint256 i; i < enclaveId.tcbLevels.length; ++i) {
uint256 size = enclaveId.tcbLevels.length;
for (uint256 i; i < size; ++i) {
EnclaveIdStruct.TcbLevel memory tcb = enclaveId.tcbLevels[i];
if (tcb.tcb.isvsvn <= quoteEnclaveReport.isvSvn) {
tcbFound = true;
Expand All @@ -231,7 +234,8 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract {
pure
returns (bool, TCBInfoStruct.TCBStatus status)
{
for (uint256 i; i < tcb.tcbLevels.length; ++i) {
uint256 size = tcb.tcbLevels.length;
for (uint256 i; i < size; ++i) {
TCBInfoStruct.TCBLevelObj memory current = tcb.tcbLevels[i];
bool pceSvnIsHigherOrGreater = pck.sgxExtension.pcesvn >= current.pcesvn;
bool cpuSvnsAreHigherOrGreater = _isCpuSvnHigherOrGreater(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,9 @@ contract PEMCertChainLib is IPEMCertChainLib {
string memory contentSlice = LibString.slice(pemData, contentStart, endPos);
string[] memory split = LibString.split(contentSlice, string(delimiter));
string memory contentStr;
uint256 size = split.length;

for (uint256 i; i < split.length; ++i) {
for (uint256 i; i < size; ++i) {
contentStr = LibString.concat(contentStr, split[i]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ library V3Parser {
}

function littleEndianDecode(bytes memory encoded) private pure returns (uint256 decoded) {
for (uint256 i; i < encoded.length; ++i) {
uint256 size = encoded.length;
for (uint256 i; i < size; ++i) {
uint256 digits = uint256(uint8(bytes1(encoded[i])));
uint256 upperDigit = digits / 16;
uint256 lowerDigit = digits % 16;
Expand Down
38 changes: 22 additions & 16 deletions packages/protocol/contracts/layer1/based/TaikoInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -205,17 +205,18 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
(BatchMetadata[] memory metas, Transition[] memory trans) =
abi.decode(_params, (BatchMetadata[], Transition[]));

require(metas.length != 0, NoBlocksToProve());
require(metas.length == trans.length, ArraySizesMismatch());
uint256 metasLength = metas.length;
require(metasLength != 0, NoBlocksToProve());
require(metasLength == trans.length, ArraySizesMismatch());

Stats2 memory stats2 = state.stats2;
require(!stats2.paused, ContractPaused());

Config memory config = pacayaConfig();
IVerifier.Context[] memory ctxs = new IVerifier.Context[](metas.length);
IVerifier.Context[] memory ctxs = new IVerifier.Context[](metasLength);

bool hasConflictingProof;
for (uint256 i; i < metas.length; ++i) {
for (uint256 i; i < metasLength; ++i) {
BatchMetadata memory meta = metas[i];

require(meta.batchId >= pacayaConfig().forkHeights.pacaya, ForkNotActivated());
Expand Down Expand Up @@ -298,8 +299,8 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {

// Emit the event
{
uint64[] memory batchIds = new uint64[](metas.length);
for (uint256 i; i < metas.length; ++i) {
uint64[] memory batchIds = new uint64[](metasLength);
for (uint256 i; i < metasLength; ++i) {
batchIds[i] = metas[i].batchId;
}

Expand All @@ -310,7 +311,7 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
_pause();
emit Paused(verifier);
} else {
_verifyBatches(config, stats2, metas.length);
_verifyBatches(config, stats2, metasLength);
}
}

Expand Down Expand Up @@ -568,13 +569,15 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
if (_blobParams.blobHashes.length != 0) {
blobHashes_ = _blobParams.blobHashes;
} else {
blobHashes_ = new bytes32[](_blobParams.numBlobs);
for (uint256 i; i < _blobParams.numBlobs; ++i) {
uint256 numBlobs = _blobParams.numBlobs;
blobHashes_ = new bytes32[](numBlobs);
for (uint256 i; i < numBlobs; ++i) {
blobHashes_[i] = blobhash(_blobParams.firstBlobIndex + i);
}
}

for (uint256 i; i < blobHashes_.length; ++i) {
uint256 bloblHashesLength = blobHashes_.length;
for (uint256 i; i < bloblHashesLength; ++i) {
require(blobHashes_[i] != 0, BlobNotFound());
}
hash_ = keccak256(abi.encode(_txListHash, blobHashes_));
Expand Down Expand Up @@ -751,6 +754,7 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
view
returns (uint64 anchorBlockId_, uint64 lastBlockTimestamp_)
{
uint256 blocksLength = _params.blocks.length;
unchecked {
if (_params.anchorBlockId == 0) {
anchorBlockId_ = uint64(block.number - 1);
Expand All @@ -774,7 +778,7 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
require(lastBlockTimestamp_ <= block.timestamp, TimestampTooLarge());

uint64 totalShift;
for (uint256 i; i < _params.blocks.length; ++i) {
for (uint256 i; i < blocksLength; ++i) {
totalShift += _params.blocks[i].timeShift;
}

Expand All @@ -799,19 +803,21 @@ abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko {
);
}

if (_params.signalSlots.length != 0) {
require(_params.signalSlots.length <= _maxSignalsToReceive, TooManySignals());
uint256 signalSlotsLength = _params.signalSlots.length;

if (signalSlotsLength != 0) {
require(signalSlotsLength <= _maxSignalsToReceive, TooManySignals());

ISignalService signalService =
ISignalService(resolve(LibStrings.B_SIGNAL_SERVICE, false));

for (uint256 i; i < _params.signalSlots.length; ++i) {
for (uint256 i; i < signalSlotsLength; ++i) {
require(signalService.isSignalSent(_params.signalSlots[i]), SignalNotSent());
}
}

require(_params.blocks.length != 0, BlockNotFound());
require(_params.blocks.length <= _maxBlocksPerBatch, TooManyBlocks());
require(blocksLength != 0, BlockNotFound());
require(blocksLength <= _maxBlocksPerBatch, TooManyBlocks());
}

// Memory-only structs ----------------------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion packages/protocol/contracts/layer1/hekla/HeklaTaikoToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ contract HeklaTaikoToken is EssentialContract, ERC20SnapshotUpgradeable, ERC20Vo
returns (bool)
{
if (recipients.length != amounts.length) revert TT_INVALID_PARAM();
for (uint256 i; i < recipients.length; ++i) {
uint256 size = recipients.length;
for (uint256 i; i < size; ++i) {
_transfer(msg.sender, recipients[i], amounts[i]);
}
return true;
Expand Down
5 changes: 3 additions & 2 deletions packages/protocol/contracts/layer1/token/TaikoToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ contract TaikoToken is TaikoTokenBase {
external
returns (bool)
{
if (recipients.length != amounts.length) revert TT_INVALID_PARAM();
for (uint256 i; i < recipients.length; ++i) {
uint256 size = recipients.length;
if (size != amounts.length) revert TT_INVALID_PARAM();
for (uint256 i; i < size; ++i) {
_transfer(msg.sender, recipients[i], amounts[i]);
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ contract Risc0Verifier is EssentialContract, IVerifier {
// First public input is the block proving program key
publicInputs[0] = blockImageId;
// All other inputs are the block program public inputs (a single 32 byte value)
for (uint256 i; i < _ctxs.length; ++i) {
uint256 size = _ctxs.length;
for (uint256 i; i < size; ++i) {
publicInputs[i + 1] = LibPublicInput.hashPublicInputs(
_ctxs[i].transition, address(this), address(0), _ctxs[i].metaHash, taikoChainId
);
Expand Down
4 changes: 3 additions & 1 deletion packages/protocol/contracts/layer1/verifiers/SP1Verifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ contract SP1Verifier is EssentialContract, IVerifier {
// First public input is the block proving program key
publicInputs[0] = blockProvingProgram;
// All other inputs are the block program public inputs (a single 32 byte value)
for (uint256 i; i < _ctxs.length; ++i) {

uint256 size = _ctxs.length;
for (uint256 i; i < size; ++i) {
publicInputs[i + 1] = LibPublicInput.hashPublicInputs(
_ctxs[i].transition, address(this), address(0), _ctxs[i].metaHash, taikoChainId
);
Expand Down
18 changes: 11 additions & 7 deletions packages/protocol/contracts/layer1/verifiers/SgxVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ contract SgxVerifier is EssentialContract, IVerifier {
external
onlyFromOwnerOrNamed(LibStrings.B_SGX_WATCHDOG)
{
for (uint256 i; i < _ids.length; ++i) {
uint256 size = _ids.length;
for (uint256 i; i < size; ++i) {
uint256 idx = _ids[i];

require(instances[idx].addr != address(0), SGX_INVALID_INSTANCE());
Expand Down Expand Up @@ -151,18 +152,18 @@ contract SgxVerifier is EssentialContract, IVerifier {
// 4 bytes + 20 bytes + 20 bytes + 65 bytes (signature) = 109
require(_proof.length == 109, SGX_INVALID_PROOF());

uint32 id = uint32(bytes4(_proof[:4]));
address oldInstance = address(bytes20(_proof[4:24]));
address newInstance = address(bytes20(_proof[24:44]));
bytes memory signature = _proof[44:];

// Collect public inputs
bytes32[] memory publicInputs = new bytes32[](_ctxs.length + 2);
uint256 size = _ctxs.length;
bytes32[] memory publicInputs = new bytes32[](size + 2);
// First public input is the current instance public key
publicInputs[0] = bytes32(uint256(uint160(oldInstance)));
publicInputs[1] = bytes32(uint256(uint160(newInstance)));

// All other inputs are the block program public inputs (a single 32 byte value)
for (uint256 i; i < _ctxs.length; ++i) {
for (uint256 i; i < size; ++i) {
// TODO(Yue): For now this assumes the new instance public key to remain the same
publicInputs[i + 2] = LibPublicInput.hashPublicInputs(
_ctxs[i].transition, address(this), newInstance, _ctxs[i].metaHash, taikoChainId
Expand All @@ -171,8 +172,10 @@ contract SgxVerifier is EssentialContract, IVerifier {

bytes32 signatureHash = keccak256(abi.encodePacked(publicInputs));
// Verify the blocks
bytes memory signature = _proof[44:];
require(oldInstance == ECDSA.recover(signatureHash, signature), SGX_INVALID_PROOF());

uint32 id = uint32(bytes4(_proof[:4]));
require(_isInstanceValid(id, oldInstance), SGX_INVALID_INSTANCE());

if (newInstance != oldInstance && newInstance != address(0)) {
Expand All @@ -187,15 +190,16 @@ contract SgxVerifier is EssentialContract, IVerifier {
private
returns (uint256[] memory ids)
{
ids = new uint256[](_instances.length);
uint256 size = _instances.length;
ids = new uint256[](size);

uint64 validSince = uint64(block.timestamp);

if (!instantValid) {
validSince += INSTANCE_VALIDITY_DELAY;
}

for (uint256 i; i < _instances.length; ++i) {
for (uint256 i; i < size; ++i) {
require(!addressRegistered[_instances[i]], SGX_ALREADY_ATTESTED());

addressRegistered[_instances[i]] = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ abstract contract SgxVerifierBase is EssentialContract {
external
onlyFromOwnerOrNamed(LibStrings.B_SGX_WATCHDOG)
{
for (uint256 i; i < _ids.length; ++i) {
uint256 size = _ids.length;
for (uint256 i; i < size; ++i) {
uint256 idx = _ids[i];

require(instances[idx].addr != address(0), SGX_INVALID_INSTANCE());
Expand All @@ -129,15 +130,16 @@ abstract contract SgxVerifierBase is EssentialContract {
internal
returns (uint256[] memory ids)
{
ids = new uint256[](_instances.length);
uint256 size = _instances.length;
ids = new uint256[](size);

uint64 validSince = uint64(block.timestamp);

if (!instantValid) {
validSince += INSTANCE_VALIDITY_DELAY;
}

for (uint256 i; i < _instances.length; ++i) {
for (uint256 i; i < size; ++i) {
require(!addressRegistered[_instances[i]], SGX_ALREADY_ATTESTED());

addressRegistered[_instances[i]] = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ abstract contract ComposeVerifier is EssentialContract, IVerifier {
onlyFromNamed(LibStrings.B_TAIKO)
{
SubProof[] memory subProofs = abi.decode(_proof, (SubProof[]));
address[] memory verifiers = new address[](subProofs.length);
uint256 size = subProofs.length;
address[] memory verifiers = new address[](size);

address verifier;

for (uint256 i; i < subProofs.length; ++i) {
for (uint256 i; i < size; ++i) {
require(subProofs[i].verifier != address(0), CV_INVALID_SUB_VERIFIER());
require(subProofs[i].verifier > verifier, CV_INVALID_SUB_VERIFIER_ORDER());

Expand Down
1 change: 0 additions & 1 deletion packages/protocol/contracts/layer2/based/TaikoAnchor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ contract TaikoAnchor is EssentialContract, IBlockHashProvider, TaikoAnchorDeprec
error L2_FORK_ERROR();
error L2_INVALID_L1_CHAIN_ID();
error L2_INVALID_L2_CHAIN_ID();
error L2_INVALID_PARAM();
error L2_INVALID_SENDER();
error L2_PUBLIC_INPUT_HASH_MISMATCH();
error L2_TOO_LATE();
Expand Down
10 changes: 7 additions & 3 deletions packages/protocol/contracts/shared/tokenvault/ERC1155Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
{
if (msg.value < _op.fee) revert VAULT_INSUFFICIENT_FEE();

for (uint256 i; i < _op.amounts.length; ++i) {
if (_op.amounts[i] == 0) revert VAULT_INVALID_AMOUNT();
{
uint256 size = _op.amounts.length;
for (uint256 i; i < size; ++i) {
if (_op.amounts[i] == 0) revert VAULT_INVALID_AMOUNT();
}
}
// Check token interface support
if (!_op.token.supportsInterface(type(IERC1155).interfaceId)) {
Expand Down Expand Up @@ -244,7 +247,8 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
IERC1155(_op.token).safeBatchTransferFrom(
msg.sender, address(this), _op.tokenIds, _op.amounts, ""
);
for (uint256 i; i < _op.tokenIds.length; ++i) {
uint256 size = _op.tokenIds.length;
for (uint256 i; i < size; ++i) {
IBridgedERC1155(_op.token).burn(_op.tokenIds[i], _op.amounts[i]);
}
} else {
Expand Down
Loading

0 comments on commit 760fb56

Please sign in to comment.