Upgradeable contract components implementation

[ngrinkevich]

Refs #802

Implements RFC5 upgradeable contract components scheme. Adds minor refactoring to reduce the amount of repeated code.

Further additions required to fully match RFC5, saving these for a separate PR:

• createGroup() method on Operator contract
• Service contract to select operator on requestRelayEntry()

[sthompson22]

Thanks for going through all of the infra config files.

Based on what I'm seeing here once this is merged keep-dev should get provisioned properly. I'll fire a relay request once I see this deployment go out.

For keep-test we'll need to be a bit more careful on rolling this out since external parties have as-is configuration files. Once keep-dev is proven to be kosher I'll make the arrangements for keep-test.

[sthompson22]

Did not mean to add that label - sorry.

[sthompson22]

It may be worth running a keep-dev deployment off of this branch before it's merged, unless someones already done it.

[pdyraga]

@ngrinkevich I think I accepted the fact of the existence of separate request counters, especially that I can't propose any better alternative, not counting a separate counter contract which I am not sure if is any better. 😉

Let's work on naming a little - use requestId everywhere consistently in the service contract and have signingId in the operator contract just like you proposed. I also think we should avoid duplicating the state about the number of groups between service and operator contract as it always hurts long-term and I do not see any security reason for doing it. Other than that, I think some events can be removed.

I left a lot of comments but most of them are about proposed renames. I think we are really close to merging it.

[pdyraga]

Shouldn't we use 1 instead of 0 for requestID? Service contract increses the counter before calling sign so if we were sending this request through the service contract, we'd start with 1.

An alternative could be to store this request under signingId = 0, that is, to do not increment requestCounter in the previous line. It could make sense since it's a special - seed - request and we may want to differentiate it as such.

[pdyraga]

Still not addressed.

[pdyraga]

/Users/piotr/go/src/github.com/keep-network/keep-core/contracts/solidity/contracts/KeepRandomBeaconServiceImplV1.sol:144:13: Warning: Return value of low-level calls not used.
            _callbacks[requestId].callbackContract.call(abi.encodeWithSignature(_callbacks[requestId].callbackMethod, entry));
            ^---------------------------------------------------------------------------------------------------------------^

[pdyraga]

No longer existing KeepGroup contract is referenced in contracts/solidity/scripts/initiate-unstake.js

Fixed in #900 in 3a4b96f

[pdyraga]

Tested genesis, works as expected:

➜  solidity git:(encryption-for-state-management) ✗ truffle exec ./scripts/genesis.js --network local
Using network 'local'.

Genesis entry successfully submitted.

Tested relay request with no callback, works as expected:

➜  solidity git:(upgradeable-contract-components) ✗ truffle exec ./scripts/request-relay-entry.js --network local
Using network 'local'.

Successfully requested relay entry with RequestId = 3

---Transaction Summary---
From:0xfa3da235947aab49d439f3bcb46effd1a7237e32
To:0x5d1a5270c7cf288e81d4f6b4276a3c8ba0d0e6b3
BlockNumber:78240
TotalGas:154360
TransactionHash:0xdf1d904144c37a9ab0e0de4a262a342c5f789cb997656ef6268e333ef402602e
--------------------------

Tested relay request with callback, works as expected:

➜  solidity git:(upgradeable-contract-components) ✗ truffle exec ./scripts/request-relay-entry-with-callback.js 0xdd5eaa61d0d0476d7653490daa22b67e7e77ab78 "callback(uint256)" 100 --network local
Using network 'local'.

Successfully requested relay entry with a callback. RequestId = 4

---Transaction Summary---
From:0xfa3da235947aab49d439f3bcb46effd1a7237e32
To:0x5d1a5270c7cf288e81d4f6b4276a3c8ba0d0e6b3
BlockNumber:78247
TotalGas:198836
TransactionHash:0x6c7c92416be57b56f199c97d56165e176f770819af37ed5b57d8052c3b918c26
--------------------------

pragma solidity >=0.4.21 <0.6.0;

contract Caller { 
    uint256 internal _lastEntry;

    function callback(uint256 entry) public {
        _lastEntry = entry;
    }
    
    function lastEntry() public view returns (uint256)
    {
        return _lastEntry;
    }
}

[pdyraga]

Things that still need to be addressed in separate PRs:

• operator contract selection
• operator contract shouldn't trigger group selection - see this conversation

[pdyraga]

I left some comments but they are all non-blocking. I'll try to address some of them in a separate PR today.