Use ecrecover instead of tx.origin

[tedwu13]

Overview

The current Orchestrator contract uses require(msg.sender == tx.origin) to prevent contracts from calling rebase. But tx.origin may be deprecated in the future. Implement one of these alternate mechanisms described here. In the article, it suggest to use the ECDSA library or use the ecrecover Ethereum function. I decided to add the ECDSA library since it has some require checks and validations. Also it is better to abstract ECDSA implementation details. However the tradeoff is increased contract size by importing a new external contract plus maintain external contracts with version changes. This is up for discussion.

Changes

• Import ECDSA from 0.7.0^ (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/solc-0.7/contracts/cryptography/ECDSA.sol)
• Lock down the version to be 0.7.6 (current solidity version)
• Created a boolean function verifyAddressOfSignature to first sign the hashed message with a prefix and then use ECDSA's recover function to compare with the msg.sender address to make sure the address is an externally owned account since only EOAs can create valid signatures

Open Questions

We would need to use web3 to grab the signature and a message hash and pass in this verifyAddressOfSignature function to verify. I'm not as familiar with this so would love get some pointers on that. I've been doing some digging with Geth and web3.eth.sign.

Documentation about ECDSA

ECDSA recover: https://docs.openzeppelin.com/contracts/3.x/api/cryptography#ECDSA-recover-bytes32-uint8-bytes32-bytes32-

Reviewers

@nithinkrishna @brandoniles

[tedwu13]

@nithinkrishna in order to verify the address of the signature, is there a front end that sends the signature/hashed_message? Not sure if this is what you are thinking about. Open for discussion

[aalavandhan]

You could directly import from openzeppelin/contracts/utils/cryptography/ECDSA.sol, rather than copy the code into the repo.

[aalavandhan]

Some background:
We copied dependency code in for UFragments and UFragmentsPolicy to preserve the storage layout (since the first version was built on solidity 0.4.24, openzeppelin v2.2 and later versions of openzeppelin altered the storage layout).I dont see us doing this ever again.

We can add external libraries as a dependency using yarn and just import required contracts directly.

[aalavandhan]

@nithinkrishna in order to verify the address of the signature, is there a front end that sends the signature/hashed_message? Not sure if this is what you are thinking about. Open for discussion

Yea this requires some discussion. @brandoniles @thegostep

• We should use EIP-712 for structured data hashing and signing?
• As for the data, we could use the previous rebase epoch?

[aalavandhan]

As far as I've seen I think the standard for library definitions is usually right at the top, before any storage variable definitions.

contract Orchestrator is Ownable {
  using ECDSA for address;
  
  // Rest of the contract
}

[thegostep]

This approach makes sense to me in short to medium term - longer term it would be great to find a way to allow smart contracts to call rebase

[thegostep]

• We should use EIP-712 for structured data hashing and signing

I don't see an advantage to using 712 here, all we need is a proof that msg.sender is an EOA so the content of the message is irrelevant