Skip to content

Commit

Permalink
fix: add data_format in hash for tw
Browse files Browse the repository at this point in the history
  • Loading branch information
@Amuhar
Amuhar committed Mar 3, 2025
1 parent af497f3 commit 7163ee2
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 50 deletions.
90 changes: 69 additions & 21 deletions contracts/0.8.9/oracle/ValidatorsExitBus.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ contract ValidatorsExitBus is AccessControlEnumerable {
using UnstructuredStorage for bytes32;

/// @dev Errors
error KeyWasNotUnpacked(uint256 keyIndex, uint256 lastUnpackedKeyIndex);
error KeyWasNotDelivered(uint256 keyIndex, uint256 lastDeliveredKeyIndex);
error ZeroAddress();
error FeeNotEnough(uint256 minFeePerRequest, uint256 requestCount, uint256 msgValue);
error InsufficientPayment(uint256 withdrawalFeePerRequest, uint256 requestCount, uint256 msgValue);
error TriggerableWithdrawalRefundFailed();
error ExitHashWasNotSubmitted();
error KeyIndexOutOfRange(uint256 keyIndex, uint256 totalItemsCount);
Expand All @@ -29,18 +29,18 @@ contract ValidatorsExitBus is AccessControlEnumerable {
address sender,
uint256 refundValue
);
event StoredExitRequestHash(
bytes32 exitRequestHash
);

// TODO: make type optimization
struct DeliveryHistory {
uint256 blockNumber;
/// @dev Key index in exit request array
uint256 lastDeliveredKeyIndex;

// TODO: timestamp
/// @dev Block timestamp
uint256 timestamp;
}
// TODO: make type optimization
struct RequestStatus {
// Total items count in report (by default type(uint32).max, update on first report unpack)
// Total items count in report (by default type(uint32).max, update on first report delivery)
uint256 totalItemsCount;
// Total processed items in report (by default 0)
uint256 deliveredItemsCount;
Expand All @@ -49,6 +49,10 @@ contract ValidatorsExitBus is AccessControlEnumerable {

DeliveryHistory[] deliverHistory;
}
struct ExitRequestData {
bytes data;
uint256 dataFormat;
}

/// Length in bytes of packed request
uint256 internal constant PACKED_REQUEST_LENGTH = 64;
Expand All @@ -67,43 +71,60 @@ contract ValidatorsExitBus is AccessControlEnumerable {
LOCATOR_CONTRACT_POSITION.setStorageAddress(addr);
}

function triggerExitHashVerify(bytes calldata data, uint256[] calldata keyIndexes) external payable {
bytes32 dataHash = keccak256(data);
RequestStatus storage requestStatus = _storageExitRequestsHashes()[dataHash];
/// @notice Triggers exits on the EL via the Withdrawal Vault contract after
/// @dev This function verifies that the hash of the provided exit request data exists in storage
// and ensures that the events for the requests specified in the `keyIndexes` array have already been delivered.
function triggerExits(ExitRequestData calldata request, uint256[] calldata keyIndexes) external payable {
RequestStatus storage requestStatus = _storageExitRequestsHashes()[keccak256(abi.encode(request.data, request.dataFormat))];
bytes calldata data = request.data;

if (requestStatus.contractVersion == 0) {
revert ExitHashWasNotSubmitted();
}

address locatorAddr = LOCATOR_CONTRACT_POSITION.getStorageAddress();
address withdrawalVaultAddr = ILidoLocator(locatorAddr).withdrawalVault();
uint256 minFee = IWithdrawalVault(withdrawalVaultAddr).getWithdrawalRequestFee();
uint256 requestsFee = keyIndexes.length * minFee;
uint256 withdrawalFee = IWithdrawalVault(withdrawalVaultAddr).getWithdrawalRequestFee();

if (msg.value < requestsFee) {
revert FeeNotEnough(minFee, keyIndexes.length, msg.value);
if (msg.value < keyIndexes.length * withdrawalFee ) {
revert InsufficientPayment(withdrawalFee, keyIndexes.length, msg.value);
}

uint256 refund = msg.value - requestsFee;
uint256 prevBalance = address(this).balance - msg.value;

uint256 lastDeliveredKeyIndex = requestStatus.deliveredItemsCount - 1;

bytes memory pubkeys;
bytes memory pubkeys = new bytes(keyIndexes.length * PUBLIC_KEY_LENGTH);

for (uint256 i = 0; i < keyIndexes.length; i++) {
if (keyIndexes[i] >= requestStatus.totalItemsCount) {
revert KeyIndexOutOfRange(keyIndexes[i], requestStatus.totalItemsCount);
}

if (keyIndexes[i] > lastDeliveredKeyIndex) {
revert KeyWasNotUnpacked(keyIndexes[i], lastDeliveredKeyIndex);
revert KeyWasNotDelivered(keyIndexes[i], lastDeliveredKeyIndex);
}

uint256 requestOffset = keyIndexes[i] * PACKED_REQUEST_LENGTH + 16;
pubkeys = bytes.concat(pubkeys, data[requestOffset:requestOffset + PUBLIC_KEY_LENGTH]);
///
/// | 3 bytes | 5 bytes | 8 bytes | 48 bytes |
/// | moduleId | nodeOpId | validatorIndex | validatorPubkey |
/// 16 bytes - part without pubkey
uint256 requestPublicKeyOffset = keyIndexes[i] * PACKED_REQUEST_LENGTH + 16;
uint256 destOffset = i * PUBLIC_KEY_LENGTH;

assembly {
let dest := add(pubkeys, add(32, destOffset))
calldatacopy(
dest,
add(data.offset, requestPublicKeyOffset),
PUBLIC_KEY_LENGTH
)
}
}

IWithdrawalVault(withdrawalVaultAddr).addFullWithdrawalRequests{value: requestsFee}(pubkeys);
IWithdrawalVault(withdrawalVaultAddr).addFullWithdrawalRequests{value: keyIndexes.length * withdrawalFee}(pubkeys);

uint256 refund = msg.value - keyIndexes.length * withdrawalFee;

if (refund > 0) {
(bool success, ) = msg.sender.call{value: refund}("");
Expand All @@ -115,6 +136,33 @@ contract ValidatorsExitBus is AccessControlEnumerable {
emit MadeRefund(msg.sender, refund);
}

assert(address(this).balance == prevBalance);
}

Check warning

Code scanning / Slither

Dangerous strict equalities Medium

ValidatorsExitBus.triggerExits(ValidatorsExitBus.ExitRequestData,uint256[]) uses a dangerous strict equality:
- assert(bool)(address(this).balance == prevBalance)

function _storeExitRequestHash(
bytes32 exitRequestHash,
uint256 totalItemsCount,
uint256 deliveredItemsCount,
uint256 contractVersion,
uint256 lastDeliveredKeyIndex
) internal {
if (deliveredItemsCount == 0) {
return;
}

mapping(bytes32 => RequestStatus) storage hashes = _storageExitRequestsHashes();

RequestStatus storage request = hashes[exitRequestHash];

request.totalItemsCount = totalItemsCount;
request.deliveredItemsCount = deliveredItemsCount;
request.contractVersion = contractVersion;
request.deliverHistory.push(DeliveryHistory({
timestamp: block.timestamp,
lastDeliveredKeyIndex: lastDeliveredKeyIndex
}));

emit StoredExitRequestHash(exitRequestHash);
}

/// Storage helpers
Expand Down
23 changes: 5 additions & 18 deletions contracts/0.8.9/oracle/ValidatorsExitBusOracle.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ contract ValidatorsExitBusOracle is BaseOracle, PausableUntil, ValidatorsExitBus
uint256 requestsCount
);

event StoredOracleTWExitRequestHash(
bytes32 exitRequestHash
);

event StoreOracleExitRequestHashStart(
bytes32 exitRequestHash
);
Expand Down Expand Up @@ -227,13 +223,13 @@ contract ValidatorsExitBusOracle is BaseOracle, PausableUntil, ValidatorsExitBus
{
_checkMsgSenderIsAllowedToSubmitData();
_checkContractVersion(contractVersion);
bytes32 dataHash = keccak256(data.data);
bytes32 dataHash = keccak256(abi.encode(data.data, data.dataFormat));
// it's a waste of gas to copy the whole calldata into mem but seems there's no way around
bytes32 reportDataHash = keccak256(abi.encode(data));
_checkConsensusData(data.refSlot, data.consensusVersion, reportDataHash);
_startProcessing();
_handleConsensusReportData(data);
_storeOracleExitRequestHash(dataHash, data, contractVersion);
_storeOracleExitRequestHash(dataHash, data.requestsCount, contractVersion);
}

/// @notice Returns the total number of validator exit requests ever processed
Expand Down Expand Up @@ -454,20 +450,11 @@ contract ValidatorsExitBusOracle is BaseOracle, PausableUntil, ValidatorsExitBus
return (moduleId << 40) | nodeOpId;
}

function _storeOracleExitRequestHash(bytes32 exitRequestHash, ReportData calldata report, uint256 contractVersion) internal {
if (report.requestsCount == 0) {
function _storeOracleExitRequestHash(bytes32 exitRequestHash, uint256 requestsCount, uint256 contractVersion) internal {
if (requestsCount == 0) {
return;
}

mapping(bytes32 => RequestStatus) storage hashes = _storageExitRequestsHashes();

RequestStatus storage request = hashes[exitRequestHash];
request.totalItemsCount = report.requestsCount;
request.deliveredItemsCount = report.requestsCount;
request.contractVersion = contractVersion;
request.deliverHistory.push(DeliveryHistory({blockNumber: block.number, lastDeliveredKeyIndex: report.requestsCount - 1}));

emit StoredOracleTWExitRequestHash(exitRequestHash);
_storeExitRequestHash(exitRequestHash, requestsCount, requestsCount, contractVersion, requestsCount - 1);
}

///
Expand Down
2 changes: 1 addition & 1 deletion test/0.8.9/oracle/accountingOracle.submitReportExtraData.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ describe("AccountingOracle.sol:submitReportExtraData", () => {
.withArgs(extraDataChunkHashes[1], extraDataChunkHashes[0]);

const tx2 = await oracleMemberSubmitExtraData(extraDataChunks[1]);
await expect(tx2).to.emit(oracle, "ExtraDataSubmitted").withArgs(report.refSlot, anyValue, anyValue);
await expect(tx2).to.emit(oracle, "ExtraDataSubmitted").withArgs(report.refSlot, , anyValue);

Check failure on line 491 in test/0.8.9/oracle/accountingOracle.submitReportExtraData.test.ts

View workflow job for this annotation

GitHub Actions / TypeScript 

Argument expression expected.

Check failure on line 491 in test/0.8.9/oracle/accountingOracle.submitReportExtraData.test.ts

View workflow job for this annotation

GitHub Actions / Hardhat 

Argument expression expected.

Check failure on line 491 in test/0.8.9/oracle/accountingOracle.submitReportExtraData.test.ts

View workflow job for this annotation

GitHub Actions / Hardhat 

Argument expression expected.
});

it("pass successfully if data hash matches", async () => {
Expand Down
6 changes: 4 additions & 2 deletions test/0.8.9/oracle/validator-exit-bus-oracle.submitReportData.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,11 @@ describe("ValidatorsExitBusOracle.sol:submitReportData", () => {
.withArgs(requests[1].moduleId, requests[1].nodeOpId, requests[1].valIndex, requests[1].valPubkey, timestamp);

const data = encodeExitRequestsDataList(requests);
const reportDataHash = ethers.keccak256(data);

await expect(tx).to.emit(oracle, "StoredOracleTWExitRequestHash").withArgs(reportDataHash);
const encodedData = ethers.AbiCoder.defaultAbiCoder().encode(["bytes", "uint256"], [data, reportData.dataFormat]);
const reportDataHash = ethers.keccak256(encodedData);

await expect(tx).to.emit(oracle, "StoredExitRequestHash").withArgs(reportDataHash);
});

it("updates processing state", async () => {
Expand Down
34 changes: 26 additions & 8 deletions test/0.8.9/oracle/validator-exit-bus-oracle.triggerExitHashVerify.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect } from "chai";
import { ZeroHash } from "ethers";
import { ethers } from "hardhat";

import { anyValue } from "@nomicfoundation/hardhat-chai-matchers/withArgs";
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";

import { HashConsensus__Harness, ValidatorsExitBus__Harness, WithdrawalVault__MockForVebo } from "typechain-types";
Expand All @@ -25,7 +26,7 @@ const PUBKEYS = [
"0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee",
];

describe("ValidatorsExitBusOracle.sol:triggerExitHashVerify", () => {
describe("ValidatorsExitBusOracle.sol:triggerExits", () => {
let consensus: HashConsensus__Harness;
let oracle: ValidatorsExitBus__Harness;
let admin: HardhatEthersSigner;
Expand Down Expand Up @@ -218,7 +219,11 @@ describe("ValidatorsExitBusOracle.sol:triggerExitHashVerify", () => {
});

it("someone submitted exit report data and triggered exit", async () => {
const tx = await oracle.triggerExitHashVerify(reportFields.data, [0, 1, 2, 3], { value: 4 });
const tx = await oracle.triggerExits(
{ data: reportFields.data, dataFormat: reportFields.dataFormat },
[0, 1, 2, 3],
{ value: 4 },
);

const pubkeys = [PUBKEYS[0], PUBKEYS[1], PUBKEYS[2], PUBKEYS[3]];
const concatenatedPubKeys = pubkeys.map((pk) => pk.replace(/^0x/, "")).join("");
Expand All @@ -228,33 +233,46 @@ describe("ValidatorsExitBusOracle.sol:triggerExitHashVerify", () => {
});

it("someone submitted exit report data and triggered exit on not sequential indexes", async () => {
const tx = await oracle.triggerExitHashVerify(reportFields.data, [0, 1, 3], { value: 3 });
const tx = await oracle.triggerExits({ data: reportFields.data, dataFormat: reportFields.dataFormat }, [0, 1, 3], {
value: 10,
});

const pubkeys = [PUBKEYS[0], PUBKEYS[1], PUBKEYS[3]];
const concatenatedPubKeys = pubkeys.map((pk) => pk.replace(/^0x/, "")).join("");
await expect(tx)
.to.emit(withdrawalVault, "AddFullWithdrawalRequestsCalled")
.withArgs("0x" + concatenatedPubKeys);

await expect(tx).to.emit(oracle, "MadeRefund").withArgs(anyValue, 7);
});

it("Not enough fee", async () => {
await expect(oracle.triggerExitHashVerify(reportFields.data, [0, 1], { value: 1 }))
.to.be.revertedWithCustomError(oracle, "FeeNotEnough")
await expect(
oracle.triggerExits({ data: reportFields.data, dataFormat: reportFields.dataFormat }, [0, 1], {
value: 1,
}),
)
.to.be.revertedWithCustomError(oracle, "InsufficientPayment")
.withArgs(1, 2, 1);
});

it("Should trigger withdrawals only for validators that were requested for voluntary exit by trusted entities earlier", async () => {
await expect(
oracle.triggerExitHashVerify(
"0x0000030000000000000000000000005a894d712b61ee6d5da473f87d9c8175c4022fd05a8255b6713dc75388b099a85514ceca78a52b9122d09aecda9010c047",
oracle.triggerExits(
{
data: "0x0000030000000000000000000000005a894d712b61ee6d5da473f87d9c8175c4022fd05a8255b6713dc75388b099a85514ceca78a52b9122d09aecda9010c047",
dataFormat: reportFields.dataFormat,
},
[0],
{ value: 2 },
),
).to.be.revertedWithCustomError(oracle, "ExitHashWasNotSubmitted");
});

it("Requested index out of range", async () => {
await expect(oracle.triggerExitHashVerify(reportFields.data, [5], { value: 2 }))
await expect(
oracle.triggerExits({ data: reportFields.data, dataFormat: reportFields.dataFormat }, [5], { value: 2 }),
)
.to.be.revertedWithCustomError(oracle, "KeyIndexOutOfRange")
.withArgs(5, 4);
});
Expand Down

0 comments on commit 7163ee2

Please sign in to comment.