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

Relay request callback #870

Merged
merged 16 commits into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
8 changes: 7 additions & 1 deletion cmd/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

crand "crypto/rand"

"github.com/ethereum/go-ethereum/common"
"github.com/keep-network/keep-core/config"
"github.com/keep-network/keep-core/pkg/beacon/relay"
"github.com/keep-network/keep-core/pkg/beacon/relay/event"
Expand Down Expand Up @@ -71,6 +72,11 @@ func relayRequest(c *cli.Context) error {
return fmt.Errorf("could not generate seed: [%v]", err)
}

// Callback contract address.
callbackContract := common.Address([20]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})
// Callback contract method signature, an be calculated as bytes4(keccak256("methodName(uint256)")
callbackMethod := ""

requestMutex := sync.Mutex{}

wait := make(chan struct{})
Expand Down Expand Up @@ -109,7 +115,7 @@ func relayRequest(c *cli.Context) error {

fmt.Printf("Requesting for a new relay entry at [%s]\n", time.Now())

provider.ThresholdRelay().RequestRelayEntry(seed).
provider.ThresholdRelay().RequestRelayEntry(seed, callbackContract, callbackMethod).
OnComplete(func(request *event.Request, err error) {
if err != nil {
fmt.Fprintf(
Expand Down
16 changes: 13 additions & 3 deletions contracts/solidity/contracts/KeepRandomBeaconImplV1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ contract KeepRandomBeaconImplV1 is Ownable {
address sender;
uint256 payment;
bytes groupPubKey;
address callbackContract;
string callbackMethod;
}

mapping(uint256 => Request) internal _requests;
Expand Down Expand Up @@ -79,7 +81,7 @@ contract KeepRandomBeaconImplV1 is Ownable {
// to trigger the creation of the first group. Requests are removed on successful
// entries so genesis entry can only be called once.
_requestCounter++;
_requests[_requestCounter] = Request(msg.sender, 0, genesisGroupPubKey);
_requests[_requestCounter] = Request(msg.sender, 0, genesisGroupPubKey, address(0), "");
}

/**
Expand All @@ -93,9 +95,11 @@ contract KeepRandomBeaconImplV1 is Ownable {
* @dev Creates a request to generate a new relay entry, which will include a
* random number (by signing the previous entry's random number).
* @param seed Initial seed random value from the client. It should be a cryptographically generated random value.
* @param callbackContract Callback contract address.
* @param callbackMethod Callback contract method signature.
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Do we expect the callbackMethod to follow a specific interface, that is, accept a single uint256 parameter? If so, can we make it clear in the docs?

* @return An uint256 representing uniquely generated relay request ID. It is also returned as part of the event.
*/
function requestRelayEntry(uint256 seed) public payable returns (uint256) {
function requestRelayEntry(uint256 seed, address callbackContract, string memory callbackMethod) public payable returns (uint256) {
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
require(
msg.value >= _minPayment,
"Payment is less than required minimum."
Expand All @@ -110,7 +114,7 @@ contract KeepRandomBeaconImplV1 is Ownable {

_requestCounter++;

_requests[_requestCounter] = Request(msg.sender, msg.value, groupPubKey);
_requests[_requestCounter] = Request(msg.sender, msg.value, groupPubKey, callbackContract, callbackMethod);

emit RelayEntryRequested(_requestCounter, msg.value, _previousEntry, seed, groupPubKey);
return _requestCounter;
Expand Down Expand Up @@ -161,6 +165,12 @@ contract KeepRandomBeaconImplV1 is Ownable {
require(_requests[requestID].groupPubKey.equalStorage(groupPubKey), "Provided group was not selected to produce entry for this request.");
require(BLS.verify(groupPubKey, abi.encodePacked(previousEntry, seed), bytes32(groupSignature)), "Group signature failed to pass BLS verification.");

address callbackContract = _requests[requestID].callbackContract;

if (callbackContract != address(0)) {
callbackContract.call(abi.encodeWithSignature(_requests[requestID].callbackMethod, groupSignature));
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
}

delete _requests[requestID];
_previousEntry = groupSignature;

Expand Down
29 changes: 28 additions & 1 deletion contracts/solidity/contracts/KeepRandomBeaconStub.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ contract KeepRandomBeaconStub is Ownable {
uint256 internal _previousEntry;
mapping (string => bool) internal _initialized;

struct Request {
address sender;
uint256 payment;
bytes groupPubKey;
address callbackContract;
string callbackMethod;
}

mapping(uint256 => Request) internal _requests;

/**
* @dev Prevent receiving ether without explicitly calling a function.
*/
Expand Down Expand Up @@ -47,9 +57,11 @@ contract KeepRandomBeaconStub is Ownable {
* @dev Stub method to simulate successful request to generate a new relay entry,
* which will include a random number (by signing the previous entry's random number).
* @param seed Initial seed random value from the client. It should be a cryptographically generated random value.
* @param callbackContract Callback contract address.
* @param callbackMethod Callback contract method signature.
* @return An uint256 representing uniquely generated relay request ID. It is also returned as part of the event.
*/
function requestRelayEntry(uint256 seed) public payable returns (uint256 requestID) {
function requestRelayEntry(uint256 seed, address callbackContract, string memory callbackMethod) public payable returns (uint256 requestID) {
requestID = _seq++;

// Return mocked data instead of interacting with relay.
Expand All @@ -58,8 +70,23 @@ contract KeepRandomBeaconStub is Ownable {

emit RelayEntryRequested(requestID, msg.value, _previousEntry, seed, groupPubKey);
emit RelayEntryGenerated(requestID, groupSignature, groupPubKey, _previousEntry);
_requests[requestID] = Request(msg.sender, msg.value, groupPubKey, callbackContract, callbackMethod);

_previousEntry = groupSignature;
return requestID;
}

/**
* @dev Stub method to perform a contract callback.
* @param requestID The request that started this generation - to tie the results back to the request.
* @param groupSignature The generated random number.
* @param groupPubKey Public key of the group that generated the threshold signature.
*/
function relayEntry(uint256 requestID, uint256 groupSignature, bytes memory groupPubKey, uint256 previousEntry, uint256 seed) public {

address callbackContract = _requests[requestID].callbackContract;
if (callbackContract != address(0)) {
callbackContract.call(abi.encodeWithSignature(_requests[requestID].callbackMethod, groupSignature));
}
}
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
}
29 changes: 29 additions & 0 deletions contracts/solidity/contracts/examples/CallbackContract.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
pragma solidity ^0.5.4;


/**
* @title CallbackContract
* @dev Example callback contract for Random Beacon.
*/
contract CallbackContract {

uint256 internal _lastEntry;

/**
* @dev Example of a callback method. Method signature can be
* calculated as bytes4(keccak256("callback(uint256)")
*/
function callback(uint256 requestResponse)
public
{
_lastEntry = requestResponse;
}

/**
* @dev Returns previously entry.
Copy link
Member

Choose a reason for hiding this comment

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

previously or previous?

*/
function lastEntry() public view returns (uint256)
{
return _lastEntry;
}
}
4 changes: 2 additions & 2 deletions contracts/solidity/test/TestKeepGroupSelection.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ contract('TestKeepGroupSelection', function(accounts) {

it("should not trigger group selection while one is in progress", async function() {
let groupSelectionStartBlock = await keepGroupImplViaProxy.ticketSubmissionStartBlock();
await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, {value: 10});
await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, "0x0000000000000000000000000000000000000000", "", {value: 10});
await keepRandomBeaconImplViaProxy.relayEntry(2, bls.nextGroupSignature, bls.groupPubKey, bls.groupSignature, bls.seed);

assert.isTrue((await keepGroupImplViaProxy.ticketSubmissionStartBlock()).eq(groupSelectionStartBlock), "Group selection start block should not be updated.");
Expand All @@ -118,7 +118,7 @@ contract('TestKeepGroupSelection', function(accounts) {
let groupSelectionStartBlock = await keepGroupImplViaProxy.ticketSubmissionStartBlock();
mineBlocks(timeoutChallenge + timeDKG + groupSize * resultPublicationBlockStep);

await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, {value: 10});
await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, "0x0000000000000000000000000000000000000000", "", {value: 10});
await keepRandomBeaconImplViaProxy.relayEntry(2, bls.nextGroupSignature, bls.groupPubKey, bls.groupSignature, bls.seed);

assert.isFalse((await keepGroupImplViaProxy.ticketSubmissionStartBlock()).eq(groupSelectionStartBlock), "Group selection start block should be updated.");
Expand Down
36 changes: 36 additions & 0 deletions contracts/solidity/test/TestKeepRandomBeaconCallback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {bls} from './helpers/data';
const Proxy = artifacts.require('./KeepRandomBeacon.sol');
const KeepRandomBeacon = artifacts.require('./KeepRandomBeaconStub.sol');
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
const CallbackContract = artifacts.require('./examples/CallbackContract.sol');

contract('TestKeepRandomBeaconCallback', function() {

let impl, proxy, keepRandomBeacon, callbackContract;

before(async () => {
impl = await KeepRandomBeacon.new();
proxy = await Proxy.new(impl.address);
keepRandomBeacon = await KeepRandomBeacon.at(proxy.address);
await keepRandomBeacon.initialize();
callbackContract = await CallbackContract.new();
});

it("should produce entry if callback contract was not provided", async function() {

await keepRandomBeacon.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "");
await keepRandomBeacon.relayEntry(0, bls.groupSignature, bls.groupPubKey, bls.previousEntry, bls.seed);

let result = await callbackContract.lastEntry();
assert.isFalse(result.eq(bls.groupSignature), "Value should not change on the callback contract.");
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
});

it("should successfully call method on a callback contract", async function() {

await keepRandomBeacon.requestRelayEntry(0, callbackContract.address, "callback(uint256)");
await keepRandomBeacon.relayEntry(1, bls.groupSignature, bls.groupPubKey, bls.previousEntry, bls.seed);

let result = await callbackContract.lastEntry();
assert.isTrue(result.eq(bls.groupSignature), "Value updated by the callback should be the same as relay entry.");
});

});
4 changes: 2 additions & 2 deletions contracts/solidity/test/TestKeepRandomBeaconStub.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ contract('TestKeepRandomBeaconStub', function(accounts) {
});

it("should be able to request relay entry and get response", async function() {
await implViaProxy.requestRelayEntry(seed, {from: account_one, value: 100});
await implViaProxy.requestRelayEntry(seed, "0x0000000000000000000000000000000000000000", "", {from: account_one, value: 100});

assert.equal((await implViaProxy.getPastEvents())[0].event, 'RelayEntryRequested', "RelayEntryRequested event should occur on the implementation contract.");
assert.equal((await implViaProxy.getPastEvents())[1].event, 'RelayEntryGenerated', "RelayEntryGenerated event should occur on the implementation contract.");

let previousRandomNumber = (await implViaProxy.getPastEvents())[1].args['requestResponse'].toString();
await increaseTimeTo(await latestTime()+duration.seconds(1));
await implViaProxy.requestRelayEntry(seed, {from: account_one, value: 100});
await implViaProxy.requestRelayEntry(seed, "0x0000000000000000000000000000000000000000", "", {from: account_one, value: 100});

assert.equal((await implViaProxy.getPastEvents())[1].args['previousEntry'].toString(), previousRandomNumber, "Previous entry should be present in the event.");
assert.notEqual((await implViaProxy.getPastEvents())[1].args['requestResponse'].toString(), previousRandomNumber, "New number should be different from previous.");
Expand Down
8 changes: 4 additions & 4 deletions contracts/solidity/test/TestKeepRandomBeaconUpgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ contract('TestKeepRandomBeaconUpgrade', function(accounts) {
relayRequestTimeout);

// Add a few calls that modify state so we can test later that eternal storage works as expected after upgrade
await implViaProxy.requestRelayEntry(0, {from: account_two, value: 100});
await implViaProxy.requestRelayEntry(0, {from: account_two, value: 100});
await implViaProxy.requestRelayEntry(0, {from: account_two, value: 100});
await implViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 100});
await implViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 100});
await implViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 100});

});

Expand All @@ -52,7 +52,7 @@ contract('TestKeepRandomBeaconUpgrade', function(accounts) {
let newVar = await impl2ViaProxy.getNewVar();
assert.equal(newVar, 1234, "Should be able to get new data from upgraded contract.");

await impl2ViaProxy.requestRelayEntry(0, {from: account_two, value: 100})
await impl2ViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 100})

assert.equal((await impl2ViaProxy.getPastEvents())[0].args['requestID'], 6, "requestID should not be reset and should continue to increment where it was left in previous implementation.");

Expand Down
8 changes: 4 additions & 4 deletions contracts/solidity/test/TestKeepRandomBeaconViaProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ contract('TestKeepRandomBeaconViaProxy', function(accounts) {
});

it("should fail to request relay entry with not enough ether", async function() {
await exceptThrow(implViaProxy.requestRelayEntry(0, {from: account_two, value: 99}));
await exceptThrow(implViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 99}));
});

it("should be able to request relay entry via implementation contract with enough ether", async function() {
await implViaProxy.requestRelayEntry(0, {from: account_two, value: 100})
await implViaProxy.requestRelayEntry(0, "0x0000000000000000000000000000000000000000", "", {from: account_two, value: 100})

assert.equal((await implViaProxy.getPastEvents())[0].event, 'RelayEntryRequested', "RelayEntryRequested event should occur on the implementation contract.");

Expand All @@ -53,7 +53,7 @@ contract('TestKeepRandomBeaconViaProxy', function(accounts) {

await web3.eth.sendTransaction({
from: account_two, value: 100, gas: 200000, to: proxy.address,
data: encodeCall('requestRelayEntry', ['uint256'], [0])
data: encodeCall('requestRelayEntry', ['uint256', 'address', 'string'], [0, "0x0000000000000000000000000000000000000000", ""])
});

assert.equal((await implViaProxy.getPastEvents())[0].event, 'RelayEntryRequested', "RelayEntryRequested event should occur on the proxy contract.");
Expand All @@ -70,7 +70,7 @@ contract('TestKeepRandomBeaconViaProxy', function(accounts) {
let amount = web3.utils.toWei('1', 'ether');
await web3.eth.sendTransaction({
from: account_two, value: amount, gas: 200000, to: proxy.address,
data: encodeCall('requestRelayEntry', ['uint256'], [0])
data: encodeCall('requestRelayEntry', ['uint256', 'address', 'string'], [0, "0x0000000000000000000000000000000000000000", ""])
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
});

// should fail to withdraw if not owner
Expand Down
2 changes: 1 addition & 1 deletion contracts/solidity/test/TestRelayEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ contract('TestRelayEntry', function() {
keepGroupStub = await KeepGroupStub.new();
await keepRandomBeaconImplViaProxy.initialize(1,1, bls.previousEntry, bls.groupPubKey, keepGroupStub.address,
relayRequestTimeout);
await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, {value: 10});
await keepRandomBeaconImplViaProxy.requestRelayEntry(bls.seed, "0x0000000000000000000000000000000000000000", "", {value: 10});
});

it("should not be able to submit invalid relay entry", async function() {
Expand Down
7 changes: 6 additions & 1 deletion pkg/beacon/relay/chain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package chain
import (
"math/big"

"github.com/ethereum/go-ethereum/common"
"github.com/keep-network/keep-core/pkg/beacon/relay/config"
"github.com/keep-network/keep-core/pkg/beacon/relay/event"
"github.com/keep-network/keep-core/pkg/beacon/relay/group"
Expand All @@ -20,7 +21,11 @@ type StakerAddress []byte
type RelayEntryInterface interface {
// RequestRelayEntry makes an on-chain request to start generation of a
// random signature. An event is generated.
RequestRelayEntry(seed *big.Int) *async.RelayRequestPromise
RequestRelayEntry(
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
seed *big.Int,
callbackContract common.Address,
callbackMethod string,
pdyraga marked this conversation as resolved.
Show resolved Hide resolved
) *async.RelayRequestPromise
// SubmitRelayEntry submits an entry in the threshold relay and returns a
// promise to track the submission result. The promise is fulfilled with
// the entry as seen on-chain, or failed if there is an error submitting
Expand Down
4 changes: 3 additions & 1 deletion pkg/chain/ethereum/ethereum.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ func (ec *ethereumChain) OnGroupRegistered(

func (ec *ethereumChain) RequestRelayEntry(
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
seed *big.Int,
callbackContract common.Address,
callbackMethod string,
) *async.RelayRequestPromise {
relayRequestPromise := &async.RelayRequestPromise{}

Expand Down Expand Up @@ -395,7 +397,7 @@ func (ec *ethereumChain) RequestRelayEntry(
}()

payment := big.NewInt(2) // FIXME hardcoded 2 gwei until we fill this in
_, err = ec.keepRandomBeaconContract.RequestRelayEntry(seed, payment)
_, err = ec.keepRandomBeaconContract.RequestRelayEntry(seed, callbackContract, callbackMethod, payment)
if err != nil {
subscription.Unsubscribe()
close(requestedEntry)
Expand Down
3 changes: 2 additions & 1 deletion pkg/chain/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sync"
"sync/atomic"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto/sha3"
relaychain "github.com/keep-network/keep-core/pkg/beacon/relay/chain"
relayconfig "github.com/keep-network/keep-core/pkg/beacon/relay/config"
Expand Down Expand Up @@ -317,7 +318,7 @@ func selectGroup(entry *big.Int, numberOfGroups int) int {
}

// RequestRelayEntry simulates calling to start the random generation process.
func (c *localChain) RequestRelayEntry(seed *big.Int) *async.RelayRequestPromise {
func (c *localChain) RequestRelayEntry(seed *big.Int, callbackContract common.Address, callbackMethod string) *async.RelayRequestPromise {
ngrinkevich marked this conversation as resolved.
Show resolved Hide resolved
promise := &async.RelayRequestPromise{}

selectedIdx := selectGroup(c.latestValue, len(c.groups))
Expand Down
12 changes: 9 additions & 3 deletions pkg/chain/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ func TestLocalRequestRelayEntry(t *testing.T) {

chainHandle := Connect(10, 4, big.NewInt(200)).ThresholdRelay()
seed := big.NewInt(42)
callbackContract := common.Address([20]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})
callbackMethod := ""

relayRequestPromise := chainHandle.RequestRelayEntry(seed)
relayRequestPromise := chainHandle.RequestRelayEntry(seed, callbackContract, callbackMethod)

done := make(chan *event.Request)
relayRequestPromise.OnSuccess(func(entry *event.Request) {
Expand Down Expand Up @@ -352,8 +354,10 @@ func TestLocalOnRelayEntryRequested(t *testing.T) {
defer subscription.Unsubscribe()

seed := big.NewInt(12345)
callbackContract := common.Address([20]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})
callbackMethod := ""

chainHandle.RequestRelayEntry(seed)
chainHandle.RequestRelayEntry(seed, callbackContract, callbackMethod)

select {
case event := <-eventFired:
Expand Down Expand Up @@ -410,7 +414,9 @@ func TestLocalOnRelayEntryRequestedUnsubscribed(t *testing.T) {

subscription.Unsubscribe()

chainHandle.RequestRelayEntry(big.NewInt(42))
callbackContract := common.Address([20]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})
callbackMethod := ""
chainHandle.RequestRelayEntry(big.NewInt(42), callbackContract, callbackMethod)

select {
case event := <-eventFired:
Expand Down