Skip to content

Commit

Permalink
rename, fix and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@novaknole
novaknole committed Oct 29, 2024
1 parent 2f6d3d9 commit d8935c2
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 18 deletions.
33 changes: 24 additions & 9 deletions contracts/src/permission/condition/extensions/RuledCondition.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ abstract contract RuledCondition is PermissionConditionUpgradeable {
super.supportsInterface(_interfaceId);
}

/// @notice Retrieves all the rules stored in this contract.
/// @return An array of `Rule` structs representing the defined rules.
/// @notice Retrieves the current rules stored in this contract.
/// @return An array of `Rule` structs representing the currently defined rules.
function getRules() public view virtual returns (Rule[] memory) {
return rules;
}

/// @notice Updates the set of rules stored in the contract.
/// @notice Updates the set of rules.
/// @dev This function deletes the current set of rules and replaces it with a new one.
/// @param _rules An new array of `Rule` structs to replace the current set of rules.
function _updateRules(Rule[] memory _rules) internal virtual {
Expand Down Expand Up @@ -149,7 +149,7 @@ abstract contract RuledCondition is PermissionConditionUpgradeable {
return uint256(value) > 0;
}

return _compare(value, Op(rule.op), comparedTo);
return _compare(value, comparedTo, Op(rule.op));
}

/// @notice Evaluates logical operations.
Expand Down Expand Up @@ -275,7 +275,12 @@ abstract contract RuledCondition is PermissionConditionUpgradeable {
return result;
}

function _compare(uint256 _a, Op _op, uint256 _b) internal pure returns (bool) {
/// @notice Compares two values based on the specified operation.
/// @param _a The first value to compare.
/// @param _b The second value to compare.
/// @param _op The operation to use for comparison.
/// @return Returns `true` if the comparison holds true.
function _compare(uint256 _a, uint256 _b, Op _op) internal pure returns (bool) {
if (_op == Op.EQ) return _a == _b;
if (_op == Op.NEQ) return _a != _b;
if (_op == Op.GT) return _a > _b;
Expand All @@ -287,18 +292,28 @@ abstract contract RuledCondition is PermissionConditionUpgradeable {

/// @notice Encodes rule indices into a uint240 value.
/// @param startingRuleIndex The index of the starting rule to evaluate.
/// @param successRuleIndex The index of the rule to evaluate if the condition is true.
/// @param failureRuleIndex The index of the rule to evaluate if the condition is false.
/// @param successRuleIndex The index of the rule to evaluate if the evaluation of `startingRuleIndex` was true.
/// @param failureRuleIndex The index of the rule to evaluate if the evaluation of `startingRuleIndex` was false.
/// @return The encoded value combining all three inputs.
function encodeRuleValue(
function encodeIfElse(
uint256 startingRuleIndex,
uint256 successRuleIndex,
uint256 failureRuleIndex
) public pure returns (uint240) {
return uint240(startingRuleIndex + (successRuleIndex << 32) + (failureRuleIndex << 64));
}

/// @notice Decodes rule indices into three uint32.
/// @notice Encodes two rule indexes into a uint240 value. Useful for logical operators such as `AND/OR/XOR` and others.
/// @param ruleIndex1 The first index to evaluate.
/// @param ruleIndex2 The second index to evaluate.
function encodeLogicalOperator(
uint256 ruleIndex1,
uint256 ruleIndex2
) public pure returns (uint240) {
return uint240(ruleIndex1 + (ruleIndex2 << 32));
}

/// @notice Decodes rule indices into three uint32.
/// @param _x The value to decode.
/// @return a The first 32-bit segment.
/// @return b The second 32-bit segment.
Expand Down
104 changes: 95 additions & 9 deletions contracts/test/permission/condition/extensions/ruled-condition.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import {
LOGIC_OP_RULE_ID,
DUMMY_PERMISSION_ID,
Op,
RULE_VALUE_RULE_ID,
} from '../../../utils/condition/condition';
import {loadFixture} from '@nomicfoundation/hardhat-network-helpers';
import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers';
import {expect} from 'chai';
import {ethers} from 'hardhat';

describe('RuledCondition', async () => {
it('it should be able to update the condition rules', async () => {
it('should be able to update the condition rules', async () => {
const {conditionMock} = await loadFixture(fixture);

await conditionMock.updateRules([
Expand All @@ -40,11 +41,20 @@ describe('RuledCondition', async () => {
expect(rules[0].permissionId).to.equal(DUMMY_PERMISSION_ID);
});

it('it should be able to eval simple rule (evaluation is true)', async () => {
it('should be able to eval simple rule (evaluation is true)', async () => {
const {deployer, daoMock, conditionMock, subConditionA} = await loadFixture(
fixture
);

await expect(
conditionMock.isGranted(
daoMock.address,
deployer.address,
DUMMY_PERMISSION_ID,
'0x'
)
).to.be.reverted;

// configure a simple rule in the condition
await conditionMock.updateRules([
{
Expand All @@ -68,7 +78,7 @@ describe('RuledCondition', async () => {
).to.be.true;
});

it('it should be able to eval simple rule (evaluation is false)', async () => {
it('should be able to eval simple rule (evaluation is false)', async () => {
const {deployer, daoMock, conditionMock, subConditionA} = await loadFixture(
fixture
);
Expand All @@ -93,7 +103,7 @@ describe('RuledCondition', async () => {
).to.be.false;
});

it('it should be able to eval complex rule (evaluation is true)', async () => {
it('should be able to eval complex rule (evaluation is true)', async () => {
const {
deployer,
daoMock,
Expand Down Expand Up @@ -128,7 +138,7 @@ describe('RuledCondition', async () => {
).to.be.true;
});

it('it should be able to eval complex rule (evaluation is false)', async () => {
it('should be able to eval complex rule (evaluation is false)', async () => {
const {
deployer,
daoMock,
Expand Down Expand Up @@ -162,7 +172,50 @@ describe('RuledCondition', async () => {
).to.be.false;
});

it('it should be able to eval rule that checks blockNumber', async () => {
it(`evaluates 'if/else' on sub-conditions and only returns true if at least one of them returns true`, async () => {
const {deployer, daoMock, subConditionA, subConditionB, conditionMock} =
await loadFixture(fixture);

// checks the block number is bigger or equal than 1
await conditionMock.updateRules(
ifAelseB(conditionMock, subConditionA, subConditionB)
);

// since both sub-conditions return false, our condition also returns false.
expect(
await conditionMock.isGranted(
daoMock.address,
deployer.address,
DUMMY_PERMISSION_ID,
'0x'
)
).to.be.false;

// This now must return true because `if` condition is true.
await subConditionA.setAnswer(true);
expect(
await conditionMock.isGranted(
daoMock.address,
deployer.address,
DUMMY_PERMISSION_ID,
'0x'
)
).to.be.true;

// This now must return true because `else` condition is true.
await subConditionA.setAnswer(false);
await subConditionB.setAnswer(true);
expect(
await conditionMock.isGranted(
daoMock.address,
deployer.address,
DUMMY_PERMISSION_ID,
'0x'
)
).to.be.true;
});

it('should be able to eval rule that checks blockNumber', async () => {
const {deployer, daoMock, conditionMock} = await loadFixture(fixture);

// checks the block number is bigger or equal than 1
Expand Down Expand Up @@ -202,7 +255,7 @@ describe('RuledCondition', async () => {
).to.be.false;
});

it('it should be able to eval rule that checks timestamp', async () => {
it('should be able to eval rule that checks timestamp', async () => {
const {deployer, daoMock, conditionMock} = await loadFixture(fixture);

// checks the timestamp is bigger or equal than 1
Expand Down Expand Up @@ -252,6 +305,39 @@ type FixtureResult = {
subConditionC: PermissionConditionMock;
};

function ifAelseB(
conditionMock: RuledConditionMock,
subConditionA: PermissionConditionMock,
subConditionB: PermissionConditionMock
) {
return [
{
id: LOGIC_OP_RULE_ID,
op: Op.IF_ELSE,
value: conditionMock.encodeIfElse(1, 2, 3),
permissionId: DUMMY_PERMISSION_ID,
},
{
id: CONDITION_RULE_ID,
op: Op.EQ,
value: subConditionA.address,
permissionId: DUMMY_PERMISSION_ID,
},
{
id: RULE_VALUE_RULE_ID,
op: Op.RET,
value: 1,
permissionId: DUMMY_PERMISSION_ID,
},
{
id: CONDITION_RULE_ID,
op: Op.EQ,
value: subConditionB.address,
permissionId: DUMMY_PERMISSION_ID,
},
];
}

function C_or_B_and_A_Rule(
conditionMock: RuledConditionMock,
subConditionA: PermissionConditionMock,
Expand All @@ -262,7 +348,7 @@ function C_or_B_and_A_Rule(
{
id: LOGIC_OP_RULE_ID,
op: Op.OR,
value: conditionMock.encodeRuleValue(1, 2, 3), // indx 1 and idx 2 encoded
value: conditionMock.encodeLogicalOperator(1, 2), // indx 1 and indx 2 encoded
permissionId: DUMMY_PERMISSION_ID,
},
{
Expand All @@ -274,7 +360,7 @@ function C_or_B_and_A_Rule(
{
id: LOGIC_OP_RULE_ID,
op: Op.AND,
value: conditionMock.encodeRuleValue(3, 4, 5), // indx 3 and idx 4 encoded
value: conditionMock.encodeLogicalOperator(3, 4), // indx 3 and indx 4 encoded
permissionId: DUMMY_PERMISSION_ID,
},
{
Expand Down

0 comments on commit d8935c2

Please sign in to comment.