Signature generalisation #376

recmo · 2018-02-07T01:52:06Z

Description

Make signatures opaque bytes with a one-byte flag
Implement different signature types:
- Current eth_sign
- EIP712
- msg.sender
- External contract
Make message hash compatible with EIP712
Adjust internal and external interfaces for this change

Motivation and Context

This tries to implement on:

ZEIP-1: Off-chain order generation by smart contracts
ZEIP-7: On-chain order generation by smart contracts

It does not try to implement, but is related to:

ZEIP-3: Cross-relayer orders
ZEIP-9: Allow generation of multiple orders with a single signature
ZEIP-10: multihash/multiformat message format.

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Change requires a change to the documentation.
Added tests to cover my changes.

coveralls · 2018-02-07T01:56:16Z

Coverage remained the same at 90.238% when pulling 8ef7087 on feature/contracts/module-refactors into 732a46a on v2-prototype.

abandeali1 · 2018-02-08T22:51:50Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol

+
+        // Signed using web3.eth_sign
+        } else if (stype == SignatureType.Ecrecover) {
+            require(signature.length == 67);


Shouldn't this be 66?

abandeali1 · 2018-02-08T22:55:15Z

packages/contracts/src/contracts/current/protocol/Exchange/ISigner_v1.sol

@@ -0,0 +1,9 @@
+pragma solidity ^0.4.19;


Remove _v1 from the filename.

abandeali1 · 2018-02-08T23:01:02Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol

-            s
+            order.orderHash ^ 0x1, // Domain separator maker/taker
+            order.taker,
+            takerSignature


The taker signature should be optional (so if takerSignature.length is 0, assume taker is msg.sender).

We also need to update a mapping to prevent replay attacks.

I personally prefer the more generalized approaches described in the ZEIP 18 comments. Maybe since we're pretty undecided on the best approach for this, we just make this a separate function for now?

abandeali1 · 2018-02-08T23:57:42Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol

+        address signer,
+        bytes signature)
+        public view
+        returns (bool valid)


Change named return to isValid.

abandeali1 · 2018-02-08T23:59:27Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol

-            v,
-            r,
-            s
+            order.orderHash ^ 0x1, // Domain separator maker/taker


If we have a different message format or add a salt, we can probably remove the separator.

abandeali1 · 2018-02-09T00:00:03Z

packages/contracts/src/contracts/current/protocol/Exchange/ISigner_v1.sol

+
+contract ISigner {
+
+    function validate(bytes32 hash, bytes signature)


why not rename this isValidSignature for consistency?

fabioberger · 2018-02-14T22:27:13Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
+
+        require(signature.length >= 1);
+        SignatureType stype = SignatureType(uint8(signature[0]));


Nit: can we rename stype to sigType? Camel-case and less ambiguous.

@recmo our convention is the thumbs up a comment once it's been addressed.

fabioberger · 2018-02-14T22:28:30Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        bytes32 s;
+
+        // Zero is always an invalid signature
+        if (stype == SignatureType.Invalid) {


Why is this case necessary?

It's not necessary, but it seemed a good idea from a defense programing perspective. Default values are usually zero. Let's not make them accidentally be some sort of signature.

fabioberger · 2018-02-14T22:31:33Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+            isValid = false;
+            return;
+
+        // Implicitely signed by caller


Typo: Implicitly

fabioberger · 2018-02-14T22:33:04Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+            return;
+
+        // Signed using web3.eth_sign
+        } else if (stype == SignatureType.Ecrecover) {


Super cool that V2 will keep backward compatibility support for EC sigs 🥇

fabioberger · 2018-02-14T22:35:10Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        // Anything else is illegal
+        } else {
+            revert();
+


Awkward new line. Can we remove?

fabioberger · 2018-02-14T22:35:31Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        private pure
+        returns (bytes32 result)
+    {
+        // require(b.length >= index + 32);


Is this meant to be commented out still?

It's commented for performance reasons. This check is more costly than the actual function. That should not concert us at the moment though, and it would be better to have more checks while we keep breaking stuff.

abandeali1 · 2018-02-14T23:20:36Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+
+        // Implicitely signed by caller
+        } else if (stype == SignatureType.Caller) {
+            require(signature.length == 1);


Can you give an example of how this would be used? It seems like msg.sender will always be order.maker in this case, which seems a bit strange to me.

abandeali1 · 2018-02-14T23:21:07Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        bytes32 s;
+
+        // Zero is always an invalid signature
+        if (stype == SignatureType.Invalid) {


abandeali1 · 2018-02-14T23:22:23Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol

+        require(order.takerTokenAmount > 0);
+        require(takerTokenCancelAmount > 0);
+        require(isValidSignature(
+            keccak256(orderSchemaHash, order.orderHash),


It seems a bit strange to me that we are always including the orderSchemaHash, even when we aren't using an EIP712 signature. What's the logic for this?

The reasoning is that EIP712 is not a standard for signing a hash, but a standard for generating a hash from structured data. Since none of the other signing methods have requirements on the hashing algorithm, we may as well use EIP712 everywhere.

In fact, we should probably use an EIP712 hash for order.orderHash.

abandeali1 · 2018-02-14T23:33:23Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+            return;
+
+        // Implicitly signed by caller
+        } else if (signatureType == SignatureType.Caller) {


Can you provide an example of how this is used? It seems to me that in the current contracts, msg.sender will always be order.maker.

The current PR excludes taker abstraction, so for a successful fillOrder, we always have msg.sender == order.taker. When taker abstract is added, this will no longer hold true, and this signature type can be used by either maker or taker if they call fillOrder.

In the current PR the cancelOrder function also takes a signature. If maker calls cancelOrder herself she can set the signature to SignatureType.Caller.

abandeali1 · 2018-02-14T23:34:19Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        bytes32 s;
+
+        // Zero is always an invalid signature
+        if (signatureType == SignatureType.Invalid) {


When would someone pass in an intentionally invalid signature? Why not just revert if the signature is malformed?

I want to reserve 0x00 as an invalid value, because it is the default value and might show up accidentally.

You are right, it should revert() instead of throwing, to be consistent with other invalid values.

Would there be value in adding a signature schema that rejects (returns false) always? It's pretty much free to add, does hurt and might be useful somehow?

abandeali1 · 2018-02-14T23:36:05Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol

@@ -113,26 +105,19 @@ contract MixinWrapperFunctions is
    /// @param orderAddresses Array of address arrays containing individual order addresses.


The length of the inputs here are no longer guaranteed to be 484 bytes.

What's the significance of 484? I updated fillOrderNoThrow to take arbitrary lengths.

Sorry, it wouldn't let me comment on the correct line for some reason. The delegatecall takes the input length, and we're still passing in 484.

abandeali1 · 2018-02-14T23:36:50Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol

-    /// @param s Array of ECDSA signature s parameters.
-    function batchFillOrdersNoThrow(
+
+    function fillOrdersUpTo(


Remove this function, we renamed to marketFillOrders.

Ignore my previous comment, looks like you accidentally renamed batchFillOrdersNoThrow to fillOrdersUpTo.

abandeali1 · 2018-02-14T23:38:22Z

packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol

    /// @return Validity of order signature.
    function isValidSignature(
+        bytes32 digest,


I don't really have a preference for digest over hash, but this arg is still named hash in ISigner.

recmo · 2018-02-15T01:39:36Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol

@@ -159,7 +165,8 @@ contract MixinExchangeCore is
    function cancelOrder(
        address[5] orderAddresses,
        uint256[6] orderValues,
-        uint256 takerTokenCancelAmount)
+        uint256 takerTokenCancelAmount,
+        bytes signature)


This signature does not include a nonce, so cancelOrders with this signature mechanism are prone to replays.

In the current implementation we don't even need a signature here since msg.sender is required to be the maker.

abandeali1 · 2018-02-15T07:08:52Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        assembly {
+            result := mload(add(b, index))
+        }
+    }


Let's add an explicit return here.

abandeali1 · 2018-02-15T07:09:25Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+                s
+            );
+            isValid = signer == recovered;
+            return;


Let's change all of these returns to return isValid for clarity.

LogvinovLeon · 2018-02-22T19:06:54Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol

@@ -100,29 +96,39 @@ contract MixinExchangeCore is
            expirationTimestampInSec: orderValues[4],
            orderHash: getOrderHash(orderAddresses, orderValues)
        });
+
+        // Validate order and maker only if first time seen
+        if (filled[order.orderHash] == 0 && cancelled[order.orderHash] == 0) {


Do we have any data on how expensive is the signature validation? Two state reads also consume some gas.

An ecrecover is at least 3000 gas (and your gas profiler actually showed it being one of the most expensive parts of the function). An sload is only 200 I believe.

Good question! ecrecover is 3000 gas + 36 for the hash + couple hundred overhead. State read is 200 gas. Let's ignore (partially) canceled orders for now and look at two scenarios:

Unfilled order: Read both filled and cancelled, then verify signature ~ 4000 gas.

Partially filled order: Read filled, short circuit, skip verify ~ 200 gas.

So depending on the ratio partial filled / new orders the expect gas cost is 4000 x + 200 (1-x) =
3800 x + 200.

Status quo is a fixed cost of about 3500. Setting these equal and solving for x gives 87%.

So if less than 87% of the fill orders are new orders, this method saves gas.

@tomhschmidt : Do we have empirical data on this?

But!

We need to optimize for the case where the order is valid. In this case, we would need to read both state variables anyway to process the order, so reading it earlier should not add gas cost. Making the optimization essentially free.

At the moment we don't optimize for this, and we read the same state again in getUnavailableTakerTokenAmount on line 124(244) and again on line 138. This is of course
redundant, as the state can not change in between.

I added a comment to remind us to optimize this.

LogvinovLeon · 2018-02-22T19:14:30Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+        // signature array with invalid type or length. We may as well make
+        // it an explicit option. This aids testing and analysis. It is
+        // also the initialization value for the enum type.
+        if (signatureType == SignatureType.Illegal) {


All those ifs add to understanding but increase the gas consumption. Will we comment them out in a production version?

I was not planning on that, and we need some way to distinguish the cases. What do you suggest instead?

A simple trick is to sort the branches by popularity.

LogvinovLeon · 2018-02-22T19:17:05Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+
+        // Signature verified by signer contract
+        } else if (signatureType == SignatureType.Contract) {
+            isValid = ISigner(signer).isValidSignature(hash, signature);


isValidSignature seems too generic. Can we change to isValid0xOrderSignature or smth longer?

I think it's intended to be generic though. I could see people using the same function for other purposes as well?

As it is, it is pretty generic. Right now we only use it for maker signature, we will also use it for taker signature and maybe controller signature, etc. These would then sign different objects, not Orders's but FillOrder's and possibly other structures.

LogvinovLeon · 2018-02-22T19:22:35Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol

        public
        returns (bool success, uint256 takerTokenFilledAmount)
    {
        bytes4 FILL_ORDER_FUNCTION_SIGNATURE = bytes4(keccak256("fillOrder(address[5],uint256[6],uint256,uint8,bytes32,bytes32)"));

+        // Input size is padded to a 4 + n * 32 byte boundary


If this is not a WIP any more we need more comments. Much more comments.
I'm unable to read that. Let's reference to some solidity documentation or other sources that can help people understand that. (Talking about the whole function. Let's also have it as a rule to have a very good and verbose explanation before every assembly trick. How much does it save, why is it absolutely neccessary? not against assembly in general, but I want other people to be able to understand that even if they're not so experienced.

Feel free to ignore it for now. It's still a prototype.

Yeah, I'm not happy with the complexity of this assembly at all. It's not even tested and probably contains bugs. It does show that there is a solution though. We can reimplement it using byte[] input so we can construct the input arguments in Solidity instead of assembly.

Added a // TODO

recmo · 2018-02-24T00:05:10Z

TODO

Tezor signatures

abandeali1

I kind of like adding more explicit support for 0xProject/ZEIPs#7, since this requires one less external call than SignatureType.Contract. Thoughts on adding SignatureType.Approved?

Also, it occurs to me that we should probably be returning the address of the signer rather than a bool in all cases. With taker abstraction, we don't always know the address of the taker in advance (and therefore can't pass in signer).

recmo · 2018-02-26T19:48:51Z

I kind of like adding more explicit support for 0xProject/ZEIPs#7, since this requires one less external call than SignatureType.Contract. Thoughts on adding SignatureType.Approved?

How about this:

contract MixinSignatureValidator {

    enum SignatureType {
        // ....
        Presigned
   }

    mapping ((hash, signer) => bool) presigned;

    function preSign(bytes32 hash, address signer, bytes signature) {
         require(signature.type != Presigned); // Not really necessary
         if(isValidSignature(hash, signer, signature)) {
              presigned[hash, signer] = true;
         }
    }

    function isValidSignature(bytes32 hash, address signer, bytes signature) {
         // ...
         if(signature.type == Presign && presigned[hash, signer])
              return true;
    }

  // ...
}

In the EIP-7 scenario the maker would call

preSign(orderHash, this, SignatureType.Caller);

and the taker would call

fillOrder(/* ... */, SignatureType.Presign);

If the goal is to create an on-chain order book, you can add a wrapper function that includes order details:

function preSignOrder(Order order) {
   preSign(orderHash(order), msg.sender, SignatureType.Caller);
   LogOnchainOrder(order);
}

recmo · 2018-02-26T19:59:34Z

Also, it occurs to me that we should probably be returning the address of the signer rather than a bool in all cases. With taker abstraction, we don't always know the address of the taker in advance (and therefore can't pass in signer).

Hmm... So far my solution was to add address taker, bytes takerSignature to the fillOrder arguments. In the case of SignatureType.Caller this is redundant. In the case of SignatureType.Ecrecover it is a bit more complex, as an invalid signature would always be accepted, but result in a random address (which will likely not have tokens and thus ultimately fail the TX, but I prefer if it fails at signature verification stage already). In the case of SignatureType.Contract and the above proposed SignatureType.Presign we absolutely need a provided address, but this could be provided in the bytes signature.

I'm not sure which is more elegant. Can we revisit this discussion when we PR the taker abstraction?

abandeali1 · 2018-02-26T20:25:35Z

Re presigning an order, why not just use a mapping of orderHash => signer?

I also don't think that presigning should actually require a signature (we can simply calculate the orderHash and use that, since maker is part of the hash).

recmo · 2018-02-26T21:16:28Z

Re presigning an order, why not just use a mapping of orderHash => signer?

I try to keep isValidSignature agnostic to its use case. Currently it supports multiple signers for the same message. This mapping would break that assumption by allowing only one signer per orderHash. Now that I think of it, that may also cause a race condition if we receive two preSigns for the same message hash with different signers.

I also don't think that presigning should actually require a signature (we can simply calculate the orderHash and use that, since maker is part of the hash).

This would make it specific to the maker signatures, it would not work for taker signatures and potential other kind of signature uses.

I'll open a separate PR for this feature, as it is quite a significant extension.

recmo · 2018-02-26T22:59:22Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol

+        // (3): (32 - len(signature)) mod 32
+        // (4): 420 + len(signature) + (32 - len(signature)) mod 32
+        //
+        // [1]: https://solidity.readthedocs.io/en/develop/abi-spec.html


Looks like this is even more complex: http://solidity.readthedocs.io/en/develop/abi-spec.html#use-of-dynamic-types

fabioberger · 2018-02-27T17:51:08Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+            r = get32(signature, 2);
+            s = get32(signature, 34);
+            recovered = ecrecover(
+                keccak256("\x19Ethereum Signed Message:\n32", hash),


We are really trying to push the community towards ethereum/EIPs#712

fabioberger · 2018-02-27T17:51:56Z

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

+
+        // Signature from Trezor hardware wallet
+        // It differs from web3.eth_sign in the encoding of message length
+        // (Bitcoin varint encoding vs ascii-decimal, the later is not


later -> latter

recmo requested review from abandeali1, BMillman19, dekz, fabioberger and LogvinovLeon as code owners February 7, 2018 01:52

recmo changed the title ~~Signature abstraction _ logic between modules (depends on PR #364)~~ Signature abstraction and refactors (depends on PR #364) Feb 7, 2018

BMillman19 added the contracts label Feb 7, 2018

recmo changed the title ~~Signature abstraction and refactors (depends on PR #364)~~ [WIP] Signature abstraction and refactors (depends on PR #364) Feb 8, 2018

recmo force-pushed the feature/contracts/module-refactors branch from 27f32f3 to 53c3971 Compare February 8, 2018 18:30

recmo changed the title ~~[WIP] Signature abstraction and refactors (depends on PR #364)~~ [WIP] Signature abstraction and refactors Feb 8, 2018

recmo force-pushed the feature/contracts/module-refactors branch from 6391dbe to c4efdae Compare February 8, 2018 19:12

recmo changed the title ~~[WIP] Signature abstraction and refactors~~ [WIP] Signature generalisation Feb 8, 2018

abandeali1 suggested changes Feb 9, 2018

View reviewed changes

abandeali1 force-pushed the v2-prototype branch from 40665cb to 732a46a Compare February 9, 2018 01:49

recmo force-pushed the feature/contracts/module-refactors branch 3 times, most recently from e8becc2 to d193b5c Compare February 9, 2018 02:02

BMillman19 added the WIP label Feb 9, 2018

recmo force-pushed the feature/contracts/module-refactors branch 3 times, most recently from 8ef7087 to 2d186f6 Compare February 13, 2018 23:30

recmo changed the title ~~[WIP] Signature generalisation~~ Signature generalisation Feb 13, 2018

recmo force-pushed the feature/contracts/module-refactors branch from 2d186f6 to d5cfe98 Compare February 14, 2018 00:41

fabioberger suggested changes Feb 14, 2018

View reviewed changes

recmo force-pushed the feature/contracts/module-refactors branch from d5cfe98 to fdc6cde Compare February 14, 2018 23:26

abandeali1 suggested changes Feb 14, 2018

View reviewed changes

recmo commented Feb 15, 2018

View reviewed changes

abandeali1 reviewed Feb 15, 2018

View reviewed changes

LogvinovLeon suggested changes Feb 22, 2018

View reviewed changes

recmo added 6 commits February 23, 2018 14:47

Add documentation

a9c810d

Spelling fixes

3f130d7

Remove unused return value

0c4b952

Verify length on EIP712 signatures

b5a0c7d

Fix documentation

a8cec97

Add Todos

43e5bd2

Add Trezor signatures

753f9b3

abandeali1 suggested changes Feb 26, 2018

View reviewed changes

Document noThrow wrapper and correct fixed array offset

0d9a464

recmo mentioned this pull request Feb 26, 2018

On-chain order generation by smart contracts 0xProject/ZEIPs#7

Closed

recmo commented Feb 26, 2018

View reviewed changes

Add offset to signature

e5e6e8c

abandeali1 approved these changes Feb 27, 2018

View reviewed changes

fabioberger approved these changes Feb 27, 2018

View reviewed changes

Fix spelling of latter

095388f

LogvinovLeon approved these changes Feb 27, 2018

View reviewed changes

dekz approved these changes Feb 28, 2018

View reviewed changes

recmo merged commit 5276fe7 into v2-prototype Mar 1, 2018

decanus mentioned this pull request Mar 5, 2018

Allow contracts to create orders on-chain DexyProject/protocol#24

Closed

decanus mentioned this pull request Mar 21, 2018

Allow contracts to create orders off-chain DexyProject/protocol#36

Open

fabioberger deleted the feature/contracts/module-refactors branch April 3, 2018 18:28

snyk-bot mentioned this pull request Mar 17, 2020

[Snyk] Fix for 1 vulnerabilities larrycameron80/0x-monorepo#15

Open

This was referenced May 10, 2022

[Snyk] Security upgrade solc from 0.4.26 to 0.5.11 larrycameron80/0x-monorepo#186

Open

[Snyk] Fix for 1 vulnerabilities larrycameron80/0x-monorepo#187

Open


		contract ISigner {

		function validate(bytes32 hash, bytes signature)

		@@ -113,26 +105,19 @@ contract MixinWrapperFunctions is
		/// @param orderAddresses Array of address arrays containing individual order addresses.

Signature generalisation #376

Signature generalisation #376

Conversation

recmo commented Feb 7, 2018 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

coveralls commented Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

recmo Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

recmo Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

But!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

recmo Feb 24, 2018 • edited Loading

Choose a reason for hiding this comment

recmo commented Feb 24, 2018 • edited Loading

TODO

abandeali1 left a comment

Choose a reason for hiding this comment

recmo commented Feb 26, 2018 • edited Loading

recmo commented Feb 26, 2018

abandeali1 commented Feb 26, 2018 • edited Loading

recmo commented Feb 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

recmo commented Feb 7, 2018 •

edited

Loading

coveralls commented Feb 7, 2018 •

edited

Loading

recmo Feb 15, 2018 •

edited

Loading

recmo Feb 23, 2018 •

edited

Loading

recmo Feb 24, 2018 •

edited

Loading

recmo commented Feb 24, 2018 •

edited

Loading

recmo commented Feb 26, 2018 •

edited

Loading

abandeali1 commented Feb 26, 2018 •

edited

Loading

recmo commented Feb 26, 2018 •

edited

Loading