Skip to content

[2/n] rename validation res helper file and var names #94

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fangting-alchemy merged 2 commits into from
Jul 16, 2024
Merged

[2/n] rename validation res helper file and var names #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79766e3
rename validation res helper file and var names
fangting-alchemy Jul 11, 2024
92cd259
rename test function name for validation result data handling
fangting-alchemy Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/account/UpgradeableModularAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet
import {PluginEntityLib} from "../helpers/PluginEntityLib.sol";
import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol";
import {SparseCalldataSegmentLib} from "../helpers/SparseCalldataSegmentLib.sol";
import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol";
import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationResHelpers.sol";
import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol";
import {IValidation} from "../interfaces/IValidation.sol";
import {IValidationHook} from "../interfaces/IValidationHook.sol";
Expand Down Expand Up @@ -405,7 +405,7 @@ contract UpgradeableModularAccount is
bytes calldata signatureSegment;
(signatureSegment, signature) = signature.getNextSegment();

uint256 validationData;
uint256 validationRes;

// Do preUserOpValidation hooks
PluginEntity[] memory preUserOpValidationHooks =
Expand Down Expand Up @@ -433,14 +433,14 @@ contract UpgradeableModularAccount is
}

(address plugin, uint32 entityId) = preUserOpValidationHooks[i].unpack();
uint256 currentValidationData =
uint256 currentValidationRes =
IValidationHook(plugin).preUserOpValidationHook(entityId, userOp, userOpHash);

if (uint160(currentValidationData) > 1) {
if (uint160(currentValidationRes) > 1) {
// If the aggregator is not 0 or 1, it is an unexpected value
revert UnexpectedAggregator(plugin, entityId, address(uint160(currentValidationData)));
revert UnexpectedAggregator(plugin, entityId, address(uint160(currentValidationRes)));
}
validationData = _coalescePreValidation(validationData, currentValidationData);
validationRes = _coalescePreValidation(validationRes, currentValidationRes);
}

// Run the user op validationFunction
Expand All @@ -452,17 +452,17 @@ contract UpgradeableModularAccount is
userOp.signature = signatureSegment.getBody();

(address plugin, uint32 entityId) = userOpValidationFunction.unpack();
uint256 currentValidationData = IValidation(plugin).validateUserOp(entityId, userOp, userOpHash);
uint256 currentValidationRes = IValidation(plugin).validateUserOp(entityId, userOp, userOpHash);

if (preUserOpValidationHooks.length != 0) {
// If we have other validation data we need to coalesce with
validationData = _coalesceValidation(validationData, currentValidationData);
validationRes = _coalesceValidation(validationRes, currentValidationRes);
} else {
validationData = currentValidationData;
validationRes = currentValidationRes;
}
}

return validationData;
return validationRes;
}

function _doRuntimeValidation(
Expand Down
20 changes: 10 additions & 10 deletions src/helpers/ValidationDataHelpers.sol → src/helpers/ValidationResHelpers.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,49 @@
pragma solidity ^0.8.25;

// solhint-disable-next-line private-vars-leading-underscore
function _coalescePreValidation(uint256 validationData1, uint256 validationData2)
function _coalescePreValidation(uint256 validationRes1, uint256 validationRes2)
pure
returns (uint256 resValidationData)
{
uint48 validUntil1 = uint48(validationData1 >> 160);
uint48 validUntil1 = uint48(validationRes1 >> 160);
if (validUntil1 == 0) {
validUntil1 = type(uint48).max;
}
uint48 validUntil2 = uint48(validationData2 >> 160);
uint48 validUntil2 = uint48(validationRes2 >> 160);
if (validUntil2 == 0) {
validUntil2 = type(uint48).max;
}
resValidationData = ((validUntil1 > validUntil2) ? uint256(validUntil2) << 160 : uint256(validUntil1) << 160);

uint48 validAfter1 = uint48(validationData1 >> 208);
uint48 validAfter2 = uint48(validationData2 >> 208);
uint48 validAfter1 = uint48(validationRes1 >> 208);
uint48 validAfter2 = uint48(validationRes2 >> 208);

resValidationData |= ((validAfter1 < validAfter2) ? uint256(validAfter2) << 208 : uint256(validAfter1) << 208);

// Once we know that the authorizer field is 0 or 1, we can safely bubble up SIG_FAIL with bitwise OR
resValidationData |= uint160(validationData1) | uint160(validationData2);
resValidationData |= uint160(validationRes1) | uint160(validationRes2);
}

// solhint-disable-next-line private-vars-leading-underscore
function _coalesceValidation(uint256 preValidationData, uint256 validationData)
function _coalesceValidation(uint256 preValidationData, uint256 validationRes)
pure
returns (uint256 resValidationData)
{
uint48 validUntil1 = uint48(preValidationData >> 160);
if (validUntil1 == 0) {
validUntil1 = type(uint48).max;
}
uint48 validUntil2 = uint48(validationData >> 160);
uint48 validUntil2 = uint48(validationRes >> 160);
if (validUntil2 == 0) {
validUntil2 = type(uint48).max;
}
resValidationData = ((validUntil1 > validUntil2) ? uint256(validUntil2) << 160 : uint256(validUntil1) << 160);

uint48 validAfter1 = uint48(preValidationData >> 208);
uint48 validAfter2 = uint48(validationData >> 208);
uint48 validAfter2 = uint48(validationRes >> 208);

resValidationData |= ((validAfter1 < validAfter2) ? uint256(validAfter2) << 208 : uint256(validAfter1) << 208);

// If prevalidation failed, bubble up failure
resValidationData |= uint160(preValidationData) == 1 ? 1 : uint160(validationData);
resValidationData |= uint160(preValidationData) == 1 ? 1 : uint160(validationRes);
}
20 changes: 10 additions & 10 deletions test/account/ValidationIntersection.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ contract ValidationIntersectionTest is AccountTestBase {
uint48 end2 = uint48(25);

oneHookPlugin.setValidationData(
_packValidationData(address(0), start1, end1), _packValidationData(address(0), start2, end2)
_packValidationRes(address(0), start1, end1), _packValidationRes(address(0), start2, end2)
);

PackedUserOperation memory userOp;
Expand All @@ -167,7 +167,7 @@ contract ValidationIntersectionTest is AccountTestBase {
vm.prank(address(entryPoint));
uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei);

assertEq(returnedValidationData, _packValidationData(address(0), start2, end1));
assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1));
}

function test_validationIntersect_timeBounds_intersect_2() public {
Expand All @@ -178,7 +178,7 @@ contract ValidationIntersectionTest is AccountTestBase {
uint48 end2 = uint48(25);

oneHookPlugin.setValidationData(
_packValidationData(address(0), start2, end2), _packValidationData(address(0), start1, end1)
_packValidationRes(address(0), start2, end2), _packValidationRes(address(0), start1, end1)
);

PackedUserOperation memory userOp;
Expand All @@ -189,7 +189,7 @@ contract ValidationIntersectionTest is AccountTestBase {
vm.prank(address(entryPoint));
uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei);

assertEq(returnedValidationData, _packValidationData(address(0), start2, end1));
assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1));
}

function test_validationIntersect_revert_unexpectedAuthorizer() public {
Expand Down Expand Up @@ -247,7 +247,7 @@ contract ValidationIntersectionTest is AccountTestBase {
address goodAuthorizer = makeAddr("goodAuthorizer");

oneHookPlugin.setValidationData(
_packValidationData(goodAuthorizer, start1, end1), _packValidationData(address(0), start2, end2)
_packValidationRes(goodAuthorizer, start1, end1), _packValidationRes(address(0), start2, end2)
);

PackedUserOperation memory userOp;
Expand All @@ -258,7 +258,7 @@ contract ValidationIntersectionTest is AccountTestBase {
vm.prank(address(entryPoint));
uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei);

assertEq(returnedValidationData, _packValidationData(goodAuthorizer, start2, end1));
assertEq(returnedValidationData, _packValidationRes(goodAuthorizer, start2, end1));
}

function test_validationIntersect_multiplePreValidationHooksIntersect() public {
Expand All @@ -270,8 +270,8 @@ contract ValidationIntersectionTest is AccountTestBase {

twoHookPlugin.setValidationData(
0, // returns OK
_packValidationData(address(0), start1, end1),
_packValidationData(address(0), start2, end2)
_packValidationRes(address(0), start1, end1),
_packValidationRes(address(0), start2, end2)
);

PackedUserOperation memory userOp;
Expand All @@ -282,7 +282,7 @@ contract ValidationIntersectionTest is AccountTestBase {
vm.prank(address(entryPoint));
uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei);

assertEq(returnedValidationData, _packValidationData(address(0), start2, end1));
assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1));
}

function test_validationIntersect_multiplePreValidationHooksSigFail() public {
Expand Down Expand Up @@ -318,7 +318,7 @@ contract ValidationIntersectionTest is AccountTestBase {
validAfter = uint48(validationData >> (48 + 160));
}

function _packValidationData(address authorizer, uint48 validAfter, uint48 validUntil)
function _packValidationRes(address authorizer, uint48 validAfter, uint48 validUntil)
internal
pure
returns (uint256)
Expand Down
6 changes: 3 additions & 3 deletions test/utils/AccountTestBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,15 @@ abstract contract AccountTestBase is OptimizedTest {
for (uint256 i = 0; i < preValidationHookData.length; ++i) {
sig = abi.encodePacked(
sig,
_packValidationDataWithIndex(
_packValidationResWithIndex(
preValidationHookData[i].index, preValidationHookData[i].validationData
)
);
}

// Index of the actual validation data is the length of the preValidationHooksRetrieved - aka
// one-past-the-end
sig = abi.encodePacked(sig, _packValidationDataWithIndex(255, validationData));
sig = abi.encodePacked(sig, _packValidationResWithIndex(255, validationData));

return sig;
}
Expand All @@ -238,7 +238,7 @@ abstract contract AccountTestBase is OptimizedTest {
}

// helper function to pack validation data with an index, according to the sparse calldata segment spec.
function _packValidationDataWithIndex(uint8 index, bytes memory validationData)
function _packValidationResWithIndex(uint8 index, bytes memory validationData)
internal
pure
returns (bytes memory)
Expand Down