Skip to content
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.

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

Agent app: EVM #580

Merged
merged 45 commits into from
Feb 15, 2019
Merged
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0d01aaf
Actor app: inherit Vault and test vault functionality
izqui Nov 15, 2018
1fa6386
Actor app: implement execute action
izqui Nov 16, 2018
7a4dd5b
Actor app: implement permissioned forwarding
izqui Nov 16, 2018
9830981
Actor app: improve tests
izqui Nov 16, 2018
7457eb3
Actor app: signatures
izqui Nov 19, 2018
af478c1
CI: travis wait on npm install
izqui Nov 19, 2018
ebb10aa
Actor app: lint and enable CI
izqui Nov 19, 2018
e132297
Add max gas for static calls
izqui Nov 26, 2018
3ad9920
chore: upgrade to web3-eth-abi@1.0.0-beta.38
sohkai Jan 26, 2019
ea9a5aa
Address review comments
izqui Jan 26, 2019
8e40798
Lint
izqui Feb 4, 2019
2aaf462
test: add comments, cosmetic changes, and more tests
sohkai Feb 7, 2019
c73d999
SignatureValidator: fix compile warnings
sohkai Feb 7, 2019
5f1291f
test: add context headers
sohkai Feb 7, 2019
f17a0f5
Metadata: add roles to arapp, remove webapp artifacts
izqui Feb 11, 2019
f6f66f7
Radspec strings
izqui Feb 11, 2019
eb95db8
Travis: allow actor coverage to fail, tracking fix in #658
izqui Feb 11, 2019
abf1c91
Prevent infinite loop by prohibiting setting itself as the designated…
izqui Feb 11, 2019
a10c62c
Radspec
izqui Feb 11, 2019
cc21b67
Actor -> Agent rename
izqui Feb 11, 2019
139ad8a
Add status to arapp and rename constants
izqui Feb 12, 2019
fe86092
Update ERC1271 implementation
izqui Feb 12, 2019
1849286
Add ERC1271 interfaceId
izqui Feb 12, 2019
1b7b735
Rename constants
izqui Feb 12, 2019
d960fee
Rename calldata
izqui Feb 12, 2019
39b6b7e
Refactor SignatureValidator.sol
izqui Feb 12, 2019
de9d4a6
Refactor signature validation tests and test EIP712 signatures
izqui Feb 12, 2019
474f0c1
Handle empty signatures and undefined signature modes
izqui Feb 12, 2019
345a9d8
Refactor SignatureValidator: handle ERC1271 checks in library
izqui Feb 13, 2019
499cdcb
Test ERC1271 signature wrapping
izqui Feb 13, 2019
1b76fb9
cosmetic: fix whitespace
sohkai Feb 13, 2019
98369d1
Merge branch 'next' into actor-app
sohkai Feb 13, 2019
a0bddef
Agent: always allow execute to transfer ETH (#651)
sohkai Feb 13, 2019
e8eb268
cosmetic: fix whitespace and add EIPs hash
sohkai Feb 13, 2019
2225408
chore: upgrade aragonOS and use test-helper's contracts
sohkai Feb 13, 2019
2ed4a2a
cosmetic: fix compile errors
sohkai Feb 13, 2019
c482981
cosmetic: fix linter
sohkai Feb 13, 2019
cd9614b
chore: fix travis allowed to fail matrix
sohkai Feb 13, 2019
970431a
test: ignore all test contracts from coverage
sohkai Feb 13, 2019
cff0689
Actor: reuse Vault tests (#668)
sohkai Feb 14, 2019
905c32d
Move agent from future-apps/ to apps/
izqui Feb 15, 2019
0d3e2ec
Refactor popFirstByte to copy to a new array
izqui Feb 15, 2019
5b7a5fe
Address last review comments
izqui Feb 15, 2019
2a293d1
Lint
izqui Feb 15, 2019
687e29d
Address very last review comments
izqui Feb 15, 2019
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
55 changes: 36 additions & 19 deletions apps/agent/contracts/SignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,13 @@ library SignatureValidator {
);
} else if (mode == SignatureMode.ERC1271) {
// Pop the mode byte before sending it down the validation chain
(bytes memory newSig,) = popFirstByte(signature); // `signature` IS CORRUPTED AFTER EXECUTING THIS
return safeIsValidSignature(signer, hash, newSig);
return safeIsValidSignature(signer, hash, popFirstByte(signature));
} else {
return false;
}
}

function ecVerify(bytes32 hash, address signer, bytes memory signature) internal pure returns (bool) {
function ecVerify(bytes32 hash, address signer, bytes memory signature) private pure returns (bool) {
(bool badSig, bytes32 r, bytes32 s, uint8 v) = unpackEcSig(signature);

if (badSig) {
Expand All @@ -62,7 +61,7 @@ library SignatureValidator {
return signer == ecrecover(hash, v, r, s);
}

function unpackEcSig(bytes memory signature) internal pure returns (bool badSig, bytes32 r, bytes32 s, uint8 v) {
function unpackEcSig(bytes memory signature) private pure returns (bool badSig, bytes32 r, bytes32 s, uint8 v) {
if (signature.length != 66) {
badSig = true;
return;
Expand All @@ -84,30 +83,28 @@ library SignatureValidator {
}
}

/**
* @dev DANGEROUS FUNCTION: Modifies input in place, making it invalid after using this function
* Memory layout: bytes in parenthesis are returned
* L = l - 1
* Input: ([l1][l2]....[l32][b1][b2]....[bn])
* Output: [00]([L1][L2]....[L32][b2]....[bn])
*/
function popFirstByte(bytes memory input) internal pure returns (bytes memory output, byte firstByte) {
function popFirstByte(bytes memory input) private pure returns (bytes memory output) {
uint256 inputLength = input.length;
require(inputLength > 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding an error reason here?


output = new bytes(inputLength - 1);
uint256 inputPointer;
uint256 outputPointer;
assembly {
let length := mload(input)
firstByte := mload(add(input, 0x20))
output := add(input, 0x01)
mstore8(input, 0x00) // Probably safe to remove, as it will most already likely be 0
mstore(output, sub(length, 0x01))
inputPointer := add(input, 0x21)
outputPointer := add(output, 0x20)
}
memcpy(outputPointer, inputPointer, inputLength);
}

function safeIsValidSignature(address validator, bytes32 hash, bytes memory signature) internal view returns (bool) {
function safeIsValidSignature(address validator, bytes32 hash, bytes memory signature) private view returns (bool) {
bytes memory data = abi.encodeWithSelector(ERC1271(validator).isValidSignature.selector, hash, signature);
bytes4 erc1271Return = safeBytes4StaticCall(validator, data, ERC1271_ISVALIDSIG_MAX_GAS);
return erc1271Return == ERC1271_RETURN_VALID_SIGNATURE;
}

function safeBytes4StaticCall(address target, bytes data, uint256 maxGas) internal view returns (bytes4 ret) {
function safeBytes4StaticCall(address target, bytes data, uint256 maxGas) private view returns (bytes4 ret) {
uint256 gasLeft = gasleft();

uint256 callGas = gasLeft > maxGas ? maxGas : gasLeft;
Expand All @@ -134,4 +131,24 @@ library SignatureValidator {

return ret;
}

// From: https://github.com/Arachnid/solidity-stringutils/blob/master/src/strings.sol
function memcpy(uint dest, uint src, uint len) private pure {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use uint256 consistently 👍

// Copy word-length chunks while possible
for(; len >= 32; len -= 32) {
assembly {
mstore(dest, mload(src))
}
dest += 32;
src += 32;
}

// Copy remaining bytes
uint mask = 256 ** (32 - len) - 1;
assembly {
let srcpart := and(mload(src), not(mask))
let destpart := and(mload(dest), mask)
mstore(dest, or(destpart, srcpart))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this could be optimized since we don't need to make sure the remaining bytes in dest are kept, but let's keep it the same as the library 👍.

Copy link
Contributor Author

@izqui izqui Feb 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation, but I would rather not modify the code

}
}
}