store execution id in transient storage

pull/5078/head
Hadrien Croubois 8 months ago
parent dc62599257
commit 50bd746ef1
  1. 16
      contracts/access/manager/AccessManager.sol
  2. 3
      test/access/manager/AccessManager.predicate.js

@ -8,6 +8,7 @@ import {IAccessManaged} from "./IAccessManaged.sol";
import {Address} from "../../utils/Address.sol"; import {Address} from "../../utils/Address.sol";
import {Context} from "../../utils/Context.sol"; import {Context} from "../../utils/Context.sol";
import {Multicall} from "../../utils/Multicall.sol"; import {Multicall} from "../../utils/Multicall.sol";
import {StorageSlot} from "../../utils/StorageSlot.sol";
import {Math} from "../../utils/math/Math.sol"; import {Math} from "../../utils/math/Math.sol";
import {Time} from "../../utils/types/Time.sol"; import {Time} from "../../utils/types/Time.sol";
@ -59,6 +60,7 @@ import {Time} from "../../utils/types/Time.sol";
* {AccessControl-renounceRole}. * {AccessControl-renounceRole}.
*/ */
contract AccessManager is Context, Multicall, IAccessManager { contract AccessManager is Context, Multicall, IAccessManager {
using StorageSlot for *;
using Time for *; using Time for *;
// Structure that stores the details for a target contract. // Structure that stores the details for a target contract.
@ -104,9 +106,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
mapping(uint64 roleId => Role) private _roles; mapping(uint64 roleId => Role) private _roles;
mapping(bytes32 operationId => Schedule) private _schedules; mapping(bytes32 operationId => Schedule) private _schedules;
// Used to identify operations that are currently being executed via {execute}. // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.AccessManager.executionId")) - 1)) & ~bytes32(uint256(0xff))
// This should be transient storage when supported by the EVM. bytes32 private constant ACCESS_MANAGER_EXECUTION_ID_STORAGE =
bytes32 private _executionId; 0xcd345b53ed666c7ef33210216f8d27ceee269613372bda4998fd5fcd86a8b100;
/** /**
* @dev Check that the caller is authorized to perform the operation. * @dev Check that the caller is authorized to perform the operation.
@ -502,14 +504,14 @@ contract AccessManager is Context, Multicall, IAccessManager {
} }
// Mark the target and selector as authorised // Mark the target and selector as authorised
bytes32 executionIdBefore = _executionId; bytes32 executionIdBefore = ACCESS_MANAGER_EXECUTION_ID_STORAGE.asBytes32().tload();
_executionId = _hashExecutionId(target, _checkSelector(data)); ACCESS_MANAGER_EXECUTION_ID_STORAGE.asBytes32().tstore(_hashExecutionId(target, _checkSelector(data)));
// Perform call // Perform call
Address.functionCallWithValue(target, data, msg.value); Address.functionCallWithValue(target, data, msg.value);
// Reset execute identifier // Reset execute identifier
_executionId = executionIdBefore; ACCESS_MANAGER_EXECUTION_ID_STORAGE.asBytes32().tstore(executionIdBefore);
return nonce; return nonce;
} }
@ -706,7 +708,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
* @dev Returns true if a call with `target` and `selector` is being executed via {executed}. * @dev Returns true if a call with `target` and `selector` is being executed via {executed}.
*/ */
function _isExecuting(address target, bytes4 selector) private view returns (bool) { function _isExecuting(address target, bytes4 selector) private view returns (bool) {
return _executionId == _hashExecutionId(target, selector); return ACCESS_MANAGER_EXECUTION_ID_STORAGE.asBytes32().tload() == _hashExecutionId(target, selector);
} }
/** /**

@ -241,7 +241,7 @@ function testAsRestrictedOperation({ callerIsTheManager: { executing, notExecuti
} }
}); });
describe('when _executionId is in storage for target and selector', function () { describe.skip('when _executionId is in storage for target and selector', function () {
beforeEach('set _executionId flag from calldata and target', async function () { beforeEach('set _executionId flag from calldata and target', async function () {
const executionId = ethers.keccak256( const executionId = ethers.keccak256(
ethers.AbiCoder.defaultAbiCoder().encode( ethers.AbiCoder.defaultAbiCoder().encode(
@ -249,6 +249,7 @@ function testAsRestrictedOperation({ callerIsTheManager: { executing, notExecuti
[this.target.target, this.calldata.substring(0, 10)], [this.target.target, this.calldata.substring(0, 10)],
), ),
); );
// Note: this testing methods doesn't work with execution id in temporary storage
await setStorageAt(this.manager.target, EXECUTION_ID_STORAGE_SLOT, executionId); await setStorageAt(this.manager.target, EXECUTION_ID_STORAGE_SLOT, executionId);
}); });

Loading…
Cancel
Save