Updated approval process

- Managers can only execute actions on identity
- Actors can execute actions outside identity
- Validated minimum approvals to not exceed available keyTypes
- Updated identity test unit to reflect changes
This commit is contained in:
Richard Ramos 2018-03-02 15:00:46 -04:00
parent 9b17fbce88
commit 9a07b52319
4 changed files with 112 additions and 80 deletions

View File

@ -12,11 +12,11 @@ contract Identity is ERC725, ERC735 {
mapping (uint256 => bytes32[]) claimsByType;
mapping (bytes32 => uint256) indexes;
mapping (uint => Transaction) txx;
mapping (uint256 => uint8) minimumApprovalsByKeyType;
mapping (uint256 => uint256) minimumApprovalsByKeyPurpose;
bytes32[] pendingTransactions;
uint nonce = 0;
address recoveryContract;
address managerReset;
address recoveryManager;
struct Transaction {
address to;
@ -54,10 +54,11 @@ contract Identity is ERC725, ERC735 {
}
modifier managerOrActor(bytes32 _key) {
require(
isKeyType(bytes32(msg.sender), MANAGEMENT_KEY) ||
isKeyType(bytes32(msg.sender), ACTION_KEY) ||
(recoverySet && msg.sender == address(recoveryContract))
isKeyType(bytes32(msg.sender), ACTION_KEY)
);
_;
}
@ -65,39 +66,42 @@ contract Identity is ERC725, ERC735 {
function Identity() public {
_addKey(bytes32(msg.sender), MANAGEMENT_KEY, 0);
minimumApprovalsByKeyType[MANAGEMENT_KEY] = 1;
minimumApprovalsByKeyType[ACTION_KEY] = 1;
minimumApprovalsByKeyPurpose[MANAGEMENT_KEY] = 1;
minimumApprovalsByKeyPurpose[ACTION_KEY] = 1;
}
function managerReset(address _newKey)
external
recoveryOnly
{
managerReset = _newKey;
_addKey(managerReset, purpose, _newType);
minimumApprovalsByKeyType[MANAGEMENT_KEY] = keysByPurpose[MANAGEMENT_KEY].length;
recoveryManager = _newKey;
_addKey(bytes32(recoveryManager), MANAGEMENT_KEY, 0);
minimumApprovalsByKeyPurpose[MANAGEMENT_KEY] = keysByPurpose[MANAGEMENT_KEY].length;
}
function processManagerReset(uint limit)
external
function processManagerReset(uint256 limit)
public
{
require(managerReset != address(0));
bytes32 newKey = bytes32(managerReset);
require(recoveryManager != address(0));
bytes32 newKey = bytes32(recoveryManager);
bytes32[] memory managers = keysByPurpose[MANAGEMENT_KEY];
uint totalManagers = managers.lenght;
uint256 totalManagers = managers.length;
if (limit == 0) {
limit = totalManagers;
}
minimumApprovalsByKeyType[MANAGEMENT_KEY] = totalManagers - limit + 1;
for (uint i = 0; i < limit; i++) {
address manager = managers[i];
minimumApprovalsByKeyPurpose[MANAGEMENT_KEY] = totalManagers - limit + 1;
for (uint256 i = 0; i < limit; i++) {
bytes32 manager = managers[i];
if (manager != newKey) {
_removeKey(manager, MANAGEMENT_KEY);
totalManagers--;
}
}
if (totalManagers == 1) {
managerReset = address(0);
recoveryManager = address(0);
}
}
@ -123,7 +127,7 @@ contract Identity is ERC725, ERC735 {
selfOnly
returns (bool success)
{
uint256 purpose = key[_oldKey].purpose;
uint256 purpose = keys[_oldKey].purpose;
_addKey(_newKey, purpose, _newType);
_removeKey(_oldKey, purpose);
return true;
@ -170,52 +174,54 @@ contract Identity is ERC725, ERC735 {
internal
returns(bool success)
{
Transaction storage trx = txx[_id];
bytes32 managerKeyHash = keccak256(_key, MANAGEMENT_KEY);
bytes32 actorKeyHash = keccak256(_key, ACTION_KEY);
uint8 approvalCount;
uint256 requiredKeyType;
uint256 approvalCount;
uint256 requiredKeyPurpose;
Approved(_id, _approve);
Debug1(_key, isKeyType(_key, ACTION_KEY));
if (trx.to == address(this)) {
requiredKeyType = MANAGEMENT_KEY;
if (keys[managerKeyHash].purpose == MANAGEMENT_KEY) {
approvalCount = _calculateApprovals(managerKeyHash, _approve, trx);
}
require(isKeyType(_key, MANAGEMENT_KEY));
bytes32 managerKeyHash = keccak256(_key, MANAGEMENT_KEY);
requiredKeyPurpose = MANAGEMENT_KEY;
approvalCount = _calculateApprovals(managerKeyHash, _approve, trx);
} else {
requiredKeyType = ACTION_KEY;
if (keys[managerKeyHash].purpose == ACTION_KEY) {
approvalCount = _calculateApprovals(actorKeyHash, _approve, trx);
}
require(isKeyType(_key, ACTION_KEY));
bytes32 actorKeyHash = keccak256(_key, ACTION_KEY);
requiredKeyPurpose = ACTION_KEY;
approvalCount = _calculateApprovals(actorKeyHash, _approve, trx);
}
if (approvalCount >= minimumApprovalsByKeyType[requiredKeyType]) {
if (approvalCount >= minimumApprovalsByKeyPurpose[requiredKeyPurpose]) {
Executed(_id, trx.to, trx.value, trx.data);
success = trx.to.call.value(trx.value)(trx.data);
}
}
event Debug1(bytes32 d, bool b);
function setMinimumApprovalsByKeyType(
uint256 _type,
uint8 _minimumApprovals
uint256 _purpose,
uint256 _minimumApprovals
)
public
selfOnly
{
Debug(_minimumApprovals, keysByPurpose[_purpose].length, 0);
require(_minimumApprovals > 0);
minimumApprovalsByKeyType[_type] = _minimumApprovals;
require(_minimumApprovals <= keysByPurpose[_purpose].length);
minimumApprovalsByKeyPurpose[_purpose] = _minimumApprovals;
}
event Debug(uint256 a, uint256 b, uint256 c);
function _calculateApprovals(
bytes32 _keyHash,
bool _approve,
Transaction storage trx
)
private
returns (uint8 approvalCount)
returns (uint256 approvalCount)
{
require(trx.approvals[_keyHash] != _approve);
@ -403,9 +409,6 @@ contract Identity is ERC725, ERC735 {
}
delete keys[keyHash];
// MUST only be done by keys of purpose 1, or the identity itself.
// TODO If its the identity itself, the approval process will determine its approval.
}
function getKey(

View File

@ -6,8 +6,8 @@ import "./Identity.sol";
contract IdentityKernel is InstanceStorage, Identity {
function initIdentity(address _caller) external {
require(minimumApprovalsByKeyType[MANAGEMENT_KEY] == 0);
require(minimumApprovalsByKeyPurpose[MANAGEMENT_KEY] == 0);
_addKey(bytes32(_caller), MANAGEMENT_KEY, 0);
minimumApprovalsByKeyType[MANAGEMENT_KEY] = 1;
minimumApprovalsByKeyPurpose[MANAGEMENT_KEY] = 1;
}
}

View File

@ -5,9 +5,9 @@ import "../identity/IdentityKernel.sol";
contract UpdatedIdentityKernel is IdentityKernel {
event TestFunctionExecuted(uint8 minApprovalsByManagementKeys);
event TestFunctionExecuted(uint256 minApprovalsByManagementKeys);
function test() public {
TestFunctionExecuted(minimumApprovalsByKeyType[MANAGEMENT_KEY]);
TestFunctionExecuted(minimumApprovalsByKeyPurpose[MANAGEMENT_KEY]);
}
}

View File

@ -65,12 +65,17 @@ contract('Identity', function(accounts) {
{from: accounts[0]}
);
await identity.execute(
identity.address,
0,
idUtils.encode.addKey(accounts[1], idUtils.purposes.MANAGEMENT, idUtils.types.ADDRESS),
{from: accounts[2]}
);
try {
await identity.execute(
identity.address,
0,
idUtils.encode.addKey(accounts[1], idUtils.purposes.MANAGEMENT, idUtils.types.ADDRESS),
{from: accounts[2]}
);
assert.fail('should have reverted before');
} catch(error) {
TestUtils.assertJump(error);
}
assert.equal(
await identity.getKeyPurpose(TestUtils.addressToBytes32(accounts[1])),
@ -115,7 +120,7 @@ contract('Identity', function(accounts) {
identity.address+".getKeyPurpose("+accounts[1]+") is not 0")
});
it("other key should not removes a key", async () => {
it("other key should not remove a key", async () => {
await identity.execute(
identity.address,
0,
@ -156,12 +161,17 @@ contract('Identity', function(accounts) {
{from: accounts[0]}
);
await identity.execute(
identity.address,
0,
idUtils.encode.removeKey(accounts[1], idUtils.purposes.ACTION),
{from: accounts[2]}
);
try {
await identity.execute(
identity.address,
0,
idUtils.encode.removeKey(accounts[1], idUtils.purposes.ACTION),
{from: accounts[2]}
);
assert.fail('should have reverted before');
} catch(error) {
TestUtils.assertJump(error);
}
assert.equal(
await identity.getKeyPurpose(TestUtils.addressToBytes32(accounts[1])),
@ -313,18 +323,17 @@ contract('Identity', function(accounts) {
"Test function was not executed");
});
it("MANAGEMENT_KEY execute arbitrary transaction", async () => {
await identity.execute(
testContractInstance.address,
0,
functionPayload,
{from: accounts[0]}
);
assert.notEqual(
await TestUtils.listenForEvent(testContractInstance.TestFunctionExecuted()),
undefined,
"Test function was not executed");
it("MANAGEMENT_KEY cannot execute arbitrary transaction", async () => {
try {
await identity.execute(
testContractInstance.address,
0,
functionPayload,
{from: accounts[0]}
);
} catch(error) {
TestUtils.assertJump(error);
}
});
it("Other keys NOT execute arbitrary transaction", async () => {
@ -342,7 +351,13 @@ contract('Identity', function(accounts) {
});
it("MANAGEMENT_KEY should send ether from contract", async () => {
it("ACTION_KEY should send ether from contract", async () => {
await identity.execute(
identity.address,
0,
idUtils.encode.addKey(accounts[1], idUtils.purposes.ACTION, idUtils.types.ADDRESS),
{from: accounts[0]}
);
// Adding funds to contract
await web3.eth.sendTransaction({from:accounts[0], to:identity.address, value: web3.toWei(0.05, "ether")})
@ -350,28 +365,35 @@ contract('Identity', function(accounts) {
const amountToSend = web3.toWei(0.01, "ether");
let idBalance0 = web3.eth.getBalance(identity.address);
let a1Balance0 = web3.eth.getBalance(accounts[1]);
let a2Balance0 = web3.eth.getBalance(accounts[2]);
await identity.execute(
accounts[1],
accounts[2],
amountToSend,
'',
{from: accounts[0]}
{from: accounts[1]}
);
let idBalance1 = web3.eth.getBalance(identity.address);
let a1Balance1 = web3.eth.getBalance(accounts[1]);
let a2Balance1 = web3.eth.getBalance(accounts[2]);
assert(idBalance1.toNumber, idBalance0.toNumber - amountToSend, "Contract did not send ether");
assert(a1Balance1.toNumber, a1Balance0.toNumber + amountToSend, accounts[1] + " did not receive ether");
assert(a2Balance1.toNumber, a2Balance0.toNumber + amountToSend, accounts[2] + " did not receive ether");
});
it("fire ExecutionRequested(uint256 indexed executionId, address indexed to, uint256 indexed value, bytes data)", async () => {
await identity.execute(
identity.address,
0,
idUtils.encode.addKey(accounts[1], idUtils.purposes.ACTION, idUtils.types.ADDRESS),
{from: accounts[0]}
);
await identity.execute(
testContractInstance.address,
0,
functionPayload,
{from: accounts[0]}
{from: accounts[1]}
);
const executionRequested = await TestUtils.listenForEvent(identity.ExecutionRequested());
@ -381,11 +403,18 @@ contract('Identity', function(accounts) {
});
it("fire Executed(uint256 indexed executionId, address indexed to, uint256 indexed value, bytes data)", async () => {
await identity.execute(
identity.address,
0,
idUtils.encode.addKey(accounts[1], idUtils.purposes.ACTION, idUtils.types.ADDRESS),
{from: accounts[0]}
);
await identity.execute(
testContractInstance.address,
0,
functionPayload,
{from: accounts[0]}
{from: accounts[1]}
);
const executed = await TestUtils.listenForEvent(identity.Executed());
@ -399,7 +428,7 @@ contract('Identity', function(accounts) {
/*
describe("setMinimumApprovalsByKeyType(uint256 _type, uint8 _minimumApprovals)", () => {
describe("setMinimumApprovalsByKeyPurpose(uint256 _type, uint8 _minimumApprovals)", () => {
it("MANAGEMENT_KEY should set minimum approvals for MANAGEMENT_KEYs", async () => {
});