Composer contract

[gulshanvasnani]

PR contains :

1. OSTComposer contract which facilitates the staker to get OSTPrime on the sidechain.
2. Unit test cases for requestStakeRequest, acceptStakeRequest, revokeStakeRequest, rejectStakeRequest and removeStakerProxy methods.

Fixes #740

[deepesh-kn]

Reviewing :🦖

[deepesh-kn]

Generally, I am not convinced that it is beneficial to track and check the nonces in the composer and the staker proxy. What is the benefit of providing and repeatedly checking it? /cc @deepesh-kn

Maybe we don't need at both places. If we don't check it in stakerProxy.stake() then still, it will work because it will fail in gateway.stake()

We can keep it in Composer's request stake function. This can ensure that the data stored is correct and we are increasing the storage with correct data only.

What do you think? cc: @schemar

[schemar]

I'm also re-reviewing 🦑

[schemar]

Generally, I am not convinced that it is beneficial to track and check the nonces in the composer and the staker proxy. What is the benefit of providing and repeatedly checking it? /cc @deepesh-kn

Maybe we don't need at both places. If we don't check it in stakerProxy.stake() then still, it will work because it will fail in gateway.stake()

We can keep it in Composer's request stake function. This can ensure that the data stored is correct and we are increasing the storage with correct data only.

What do you think? cc: @schemar

Since only the composer can call stake on the staker proxy, this would work. The nonce on the gateway cannot change from somewhere else, so a check on the composer makes sense 👍

[deepesh-kn]

I think you don't need the storage variable stakeRequestHashes. Instead, you could use the created stake hash to check if a stake request is already stored at that hash. That saves storage and reduces the possibilities for errors.

Interesting point. I think we can take this up in the next sprint. There are a few more enhancements/optimizations that we can think of, we will create new tickets.

[0xsarvesh]

There are a few more enhancements/optimizations that we can think of, we will create new tickets.

I didn't understand the part, Why it's not needed? 🤔

How to ensure that only one stake request is allowed to be opened in the ost composer for a given gateway(before accept stake)? it's currently ensured by stakeRequestHashes[msg.sender][address(_gateway)] == bytes32(0), on L186

[schemar]

There are a few more enhancements/optimizations that we can think of, we will create new tickets.

I didn't understand the part, Why it's not needed? 🤔

How to ensure that only one stake request is allowed to be opened in the ost composer for a given gateway(before accept stake)? it's currently ensured by stakeRequestHashes[msg.sender][address(_gateway)] == bytes32(0), on L186

You can assert the same like so (after building the hash):

    stakeRequestHash_ = hashStakeRequest(
        msg.sender,
        address(_gateway),
        _amount,
        _beneficiary,
        _gasPrice,
        _gasLimit,
        _nonce
    );

    require(
        stakeRequests[stakeRequestHash_].staker == address(0),
        "Request for this staker at this gateway is already in process."
    );

[0xsarvesh]

There are a few more enhancements/optimizations that we can think of, we will create new tickets.

I didn't understand the part, Why it's not needed? 🤔 How to ensure that only one stake request is allowed to be opened in the ost composer for a given gateway(before accept stake)? it's currently ensured by stakeRequestHashes[msg.sender][address(_gateway)] == bytes32(0), on L186

You can assert the same like so (after building the hash):

    stakeRequestHash_ = hashStakeRequest(
        msg.sender,
        address(_gateway),
        _amount,
        _beneficiary,
        _gasPrice,
        _gasLimit,
        _nonce
    );

    require(
        stakeRequests[stakeRequestHash_].staker == address(0),
        "Request for this staker at this gateway is already in process."
    );

Stake request hash will be different if any parameter will change like amount, gasPrice, gasLimit in the next request.