Skip to content

Commit a5cfe0c

Browse files
alcuecaalcueca
committed
tweak: store the plan details in EmergencyBrake
1 parent 13ec941 commit a5cfe0c

File tree

3 files changed

+66
-48
lines changed

3 files changed

+66
-48
lines changed

‎contracts/utils/EmergencyBrake.sol‎

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ interface IEmergencyBrake {
1212

1313
function plan(address target, Permission[] calldata permissions) external returns (bytes32 txHash);
1414
function cancel(bytes32 txHash) external;
15-
function execute(address target, Permission[] calldata permissions) external;
16-
function restore(address target, Permission[] calldata permissions) external;
15+
function execute(bytes32 txHash) external;
16+
function restore(bytes32 txHash) external;
1717
function terminate(bytes32 txHash) external;
1818
}
1919

@@ -28,13 +28,19 @@ interface IEmergencyBrake {
2828
contract EmergencyBrake is AccessControl, IEmergencyBrake {
2929
enum State {UNPLANNED, PLANNED, EXECUTED}
3030

31+
struct Plan {
32+
State state;
33+
address target;
34+
bytes permissions;
35+
}
36+
3137
event Planned(bytes32 indexed txHash, address indexed target);
3238
event Cancelled(bytes32 indexed txHash);
3339
event Executed(bytes32 indexed txHash, address indexed target);
3440
event Restored(bytes32 indexed txHash, address indexed target);
3541
event Terminated(bytes32 indexed txHash);
3642

37-
mapping (bytes32 => State) public plans;
43+
mapping (bytes32 => Plan) public plans;
3844

3945
constructor(address planner, address executor) AccessControl() {
4046
_grantRole(IEmergencyBrake.plan.selector, planner);
@@ -51,6 +57,8 @@ contract EmergencyBrake is AccessControl, IEmergencyBrake {
5157
external override auth
5258
returns (bytes32 txHash)
5359
{
60+
txHash = keccak256(abi.encode(target, permissions));
61+
require(plans[txHash].state == State.UNPLANNED, "Emergency already planned for.");
5462

5563
// Removing or granting ROOT permissions is out of bounds for EmergencyBrake
5664
for (uint256 i = 0; i < permissions.length; i++){
@@ -61,69 +69,78 @@ contract EmergencyBrake is AccessControl, IEmergencyBrake {
6169
);
6270
}
6371
}
64-
txHash = keccak256(abi.encode(target, permissions));
65-
require(plans[txHash] == State.UNPLANNED, "Emergency already planned for.");
66-
plans[txHash] = State.PLANNED;
72+
73+
plans[txHash] = Plan({
74+
state: State.PLANNED,
75+
target: target,
76+
permissions: abi.encode(permissions)
77+
});
6778
emit Planned(txHash, target);
6879
}
6980

7081
/// @dev Erase a planned access removal transaction
7182
function cancel(bytes32 txHash)
7283
external override auth
7384
{
74-
require(plans[txHash] == State.PLANNED, "Emergency not planned for.");
75-
plans[txHash] = State.UNPLANNED;
85+
require(plans[txHash].state == State.PLANNED, "Emergency not planned for.");
86+
delete plans[txHash];
7687
emit Cancelled(txHash);
7788
}
7889

7990
/// @dev Execute an access removal transaction
80-
function execute(address target, Permission[] calldata permissions)
91+
function execute(bytes32 txHash)
8192
external override auth
8293
{
83-
bytes32 txHash = keccak256(abi.encode(target, permissions));
84-
require(plans[txHash] == State.PLANNED, "Emergency not planned for.");
85-
plans[txHash] = State.EXECUTED;
94+
Plan memory plan_ = plans[txHash];
95+
require(plan_.state == State.PLANNED, "Emergency not planned for.");
96+
plans[txHash].state = State.EXECUTED;
8697

87-
for (uint256 i = 0; i < permissions.length; i++){
98+
Permission[] memory permissions_ = abi.decode(plan_.permissions, (Permission[]));
99+
100+
for (uint256 i = 0; i < permissions_.length; i++){
88101
// AccessControl.sol doesn't revert if revoking permissions that haven't been granted
89102
// If we don't check, planner and executor can collude to gain access to contacts
90-
for (uint256 j = 0; j < permissions[i].signatures.length; j++){
91-
AccessControl contact = AccessControl(permissions[i].contact);
92-
bytes4 permission = permissions[i].signatures[j];
103+
Permission memory permission_ = permissions_[i];
104+
for (uint256 j = 0; j < permission_.signatures.length; j++){
105+
AccessControl contact = AccessControl(permission_.contact);
106+
bytes4 signature_ = permission_.signatures[j];
93107
require(
94-
contact.hasRole(permission, target),
108+
contact.hasRole(signature_, plan_.target),
95109
"Permission not found"
96110
);
97-
contact.revokeRole(permission, target);
111+
contact.revokeRole(signature_, plan_.target);
98112
}
99113
}
100-
emit Executed(txHash, target);
114+
emit Executed(txHash, plan_.target);
101115
}
102116

103117
/// @dev Restore the orchestration from an isolated target
104-
function restore(address target, Permission[] calldata permissions)
118+
function restore(bytes32 txHash)
105119
external override auth
106120
{
107-
bytes32 txHash = keccak256(abi.encode(target, permissions));
108-
require(plans[txHash] == State.EXECUTED, "Emergency plan not executed.");
109-
plans[txHash] = State.PLANNED;
110-
111-
for (uint256 i = 0; i < permissions.length; i++){
112-
for (uint256 j = 0; j < permissions[i].signatures.length; j++){
113-
AccessControl contact = AccessControl(permissions[i].contact);
114-
bytes4 permission = permissions[i].signatures[j];
115-
contact.grantRole(permission, target);
121+
Plan memory plan_ = plans[txHash];
122+
require(plan_.state == State.EXECUTED, "Emergency plan not executed.");
123+
plans[txHash].state = State.PLANNED;
124+
125+
Permission[] memory permissions_ = abi.decode(plan_.permissions, (Permission[]));
126+
127+
for (uint256 i = 0; i < permissions_.length; i++){
128+
Permission memory permission_ = permissions_[i];
129+
for (uint256 j = 0; j < permission_.signatures.length; j++){
130+
AccessControl contact = AccessControl(permission_.contact);
131+
bytes4 signature_ = permission_.signatures[j];
132+
contact.grantRole(signature_, plan_.target);
116133
}
117134
}
118-
emit Restored(txHash, target);
135+
emit Restored(txHash, plan_.target);
119136
}
120137

121138
/// @dev Remove the restoring option from an isolated target
122139
function terminate(bytes32 txHash)
123140
external override auth
124141
{
125-
require(plans[txHash] == State.EXECUTED, "Emergency plan not executed.");
126-
plans[txHash] = State.UNPLANNED;
142+
require(plans[txHash].state == State.EXECUTED, "Emergency plan not executed.");
143+
delete plans[txHash];
127144
emit Terminated(txHash);
128145
}
129146
}

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@yield-protocol/utils-v2",
3-
"version": "2.4.1",
3+
"version": "2.4.2",
44
"description": "Yield v2 utility contracts",
55
"author": "Yield Inc.",
66
"files": [

‎test/006_emergency_brake.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,10 @@ describe("EmergencyBrake", async function () {
8787
"Emergency not planned for."
8888
);
8989
await expect(
90-
brake.connect(executorAcc).execute(target, permissions)
90+
brake.connect(executorAcc).execute(txHash)
9191
).to.be.revertedWith("Emergency not planned for.");
9292
await expect(
93-
brake.connect(plannerAcc).restore(target, permissions)
93+
brake.connect(plannerAcc).restore(txHash)
9494
).to.be.revertedWith("Emergency plan not executed.");
9595
await expect(
9696
brake.connect(plannerAcc).terminate(txHash)
@@ -123,7 +123,7 @@ describe("EmergencyBrake", async function () {
123123
"Planned"
124124
);
125125

126-
expect(await brake.plans(txHash)).to.equal(state.PLANNED);
126+
expect((await brake.plans(txHash)).state).to.equal(state.PLANNED);
127127
});
128128

129129
describe("with a planned emergency", async () => {
@@ -155,12 +155,12 @@ describe("EmergencyBrake", async function () {
155155
"Cancelled"
156156
);
157157
// .withArgs(txHash, target, contacts, signatures)
158-
expect(await brake.plans(txHash)).to.equal(state.UNPLANNED);
158+
expect((await brake.plans(txHash)).state).to.equal(state.UNPLANNED);
159159
});
160160

161161
it("cant't restore or terminate a plan that hasn't been executed", async () => {
162162
await expect(
163-
brake.connect(plannerAcc).restore(target, permissions)
163+
brake.connect(plannerAcc).restore(txHash)
164164
).to.be.revertedWith("Emergency plan not executed.");
165165
await expect(
166166
brake.connect(plannerAcc).terminate(txHash)
@@ -169,7 +169,7 @@ describe("EmergencyBrake", async function () {
169169

170170
it("only the executor can execute", async () => {
171171
await expect(
172-
brake.connect(plannerAcc).execute(target, permissions)
172+
brake.connect(plannerAcc).execute(txHash)
173173
).to.be.revertedWith("Access denied");
174174
});
175175

@@ -178,39 +178,40 @@ describe("EmergencyBrake", async function () {
178178
{ contact: contact1.address, signatures: [MINT, BURN] },
179179
{ contact: contact2.address, signatures: [MINT, BURN] },
180180
];
181+
const txHash = await brake.connect(plannerAcc).callStatic.plan(target, permissions); // GEt the txHash
181182
await brake.connect(plannerAcc).plan(target, permissions); // It can be planned, because permissions could be different at execution time
182183
await expect(
183-
brake.connect(executorAcc).execute(target, permissions)
184+
brake.connect(executorAcc).execute(txHash)
184185
).to.be.revertedWith("Permission not found");
185186
});
186187

187188
it("plans can be executed", async () => {
188189
expect(
189-
await brake.connect(executorAcc).execute(target, permissions)
190+
await brake.connect(executorAcc).execute(txHash)
190191
).to.emit(brake, "Executed");
191192

192193
expect(await contact1.hasRole(MINT, target)).to.be.false;
193194
expect(await contact1.hasRole(BURN, target)).to.be.false;
194195
expect(await contact2.hasRole(APPROVE, target)).to.be.false;
195196
expect(await contact2.hasRole(TRANSFER, target)).to.be.false;
196197

197-
expect(await brake.plans(txHash)).to.equal(state.EXECUTED);
198+
expect((await brake.plans(txHash)).state).to.equal(state.EXECUTED);
198199
});
199200

200201
describe("with an executed emergency plan", async () => {
201202
beforeEach(async () => {
202-
await brake.connect(executorAcc).execute(target, permissions);
203+
await brake.connect(executorAcc).execute(txHash);
203204
});
204205

205206
it("the same emergency plan cant't executed twice", async () => {
206207
await expect(
207-
brake.connect(executorAcc).execute(target, permissions)
208+
brake.connect(executorAcc).execute(txHash)
208209
).to.be.revertedWith("Emergency not planned for.");
209210
});
210211

211212
it("only the planner can restore or terminate", async () => {
212213
await expect(
213-
brake.connect(executorAcc).restore(target, permissions)
214+
brake.connect(executorAcc).restore(txHash)
214215
).to.be.revertedWith("Access denied");
215216
await expect(
216217
brake.connect(executorAcc).terminate(txHash)
@@ -219,15 +220,15 @@ describe("EmergencyBrake", async function () {
219220

220221
it("state can be restored", async () => {
221222
expect(
222-
await brake.connect(plannerAcc).restore(target, permissions)
223+
await brake.connect(plannerAcc).restore(txHash)
223224
).to.emit(brake, "Restored");
224225

225226
expect(await contact1.hasRole(MINT, target)).to.be.true;
226227
expect(await contact1.hasRole(BURN, target)).to.be.true;
227228
expect(await contact2.hasRole(APPROVE, target)).to.be.true;
228229
expect(await contact2.hasRole(TRANSFER, target)).to.be.true;
229230

230-
expect(await brake.plans(txHash)).to.equal(state.PLANNED);
231+
expect((await brake.plans(txHash)).state).to.equal(state.PLANNED);
231232
});
232233

233234
it("target can be terminated", async () => {
@@ -236,7 +237,7 @@ describe("EmergencyBrake", async function () {
236237
"Terminated"
237238
);
238239

239-
expect(await brake.plans(txHash)).to.equal(state.UNPLANNED);
240+
expect((await brake.plans(txHash)).state).to.equal(state.UNPLANNED);
240241
});
241242
});
242243
});

0 commit comments

Comments
 (0)