Add new version of transactionPermissionContract with gasprice support

[phahulin]

Closes #57

Add new possible transactionPermissionContract.contractVersion() == 0xfffffffffffffffe that supports filtering txs based on gas price by calling contract's method allowedTxTypes with the following signature

function allowedTxTypes(
    address sender,
    address to,
    uint256 value,
    uint256 gasPrice
)
public returns (uint32, bool)

The meaning of the returned value is the same as in the current version (2) of transactionPermissionContract: uint32 part contains permission bits described in wiki; bool part tells if returned value can be cached based solely on sender (in our case this should probably be false).

[DemiMarie]

This looks good, assuming that it compiles. That said, we really really really need a test suite and CI.

[vkomenda]

Could you split it into 16-bit half-words for readability like so: 0xffff_ffff_ffff_fffe? Also, why is it not 3, for example? If you wanted an arbitrary difficult to come up with version number, you could have something more random looking. Maybe hexspeak? Like 0xdeadbeefbaadf00d?

[phahulin]

My intention was to make it easier to pull from upstream. If parity introduces a new version of permissions contract, they will most likely pick number 3. So I chose a number from the opposite end of the scale (0xfffffffffffffffe == u64(-2)).

[vkomenda]

I mean something like

let data_decoder = match version_u64 {
    2 => Some(transact_acl::functions::allowed_tx_types::call(sender, to, value)),
    -1 => Some(transact_acl_gas_price::functions::allowed_tx_types::call(sender, to, value, gas_price)),
    _ => None,
};
if let Some(data, decoder) = data_decoder {
    client.call_contract(BlockId::Hash(*parent_hash), contract_address, data) ... // etc
} else {
    (tx_permissions::NONE, true)
}

[phahulin]

Seems to have the same problem, this time in form

error[E0308]: match arms have incompatible types
...
expected struct `tx_filter::transact_acl::functions::allowed_tx_types::Decoder`, found struct `tx_filter::transact_acl_gas_price::functions::allowed_tx_types::Decoder`

I tried a couple of other variants but they all don't work because of incompatible types.
Sorry if I'm missing something

[vkomenda]

You are right. There is a problem with the dynamic type of the [decoder function] (https://github.com/paritytech/ethabi/blob/c300678e0fa8b42bc3d54d876f2e85f37b9a4983/derive/src/function.rs#L166) derived by use_contract. Even though Decoder types in both branches implement FunctionOutputDecoder<Output = (U256, bool)>, the latter is a trait. Returning trait objects from match would require heap allocation because the compiler doesn't allow returning unwrapped trait objects.

It's better to return the result of the decoder function of type (U256, bool) from match. This is closer to what you already implemented.

[vkomenda]

There is too much repetition with the 2 branch. You might get (data, decoder) first by matching on the version, and then call client.call_contract of that pair.

[phahulin]

Could you please expand on this, I'm not sure how to do it. So, to do this, I'd need to put client.call_contract(...) into either a new function or a new closure, right? In both cases I need to specify type for decoder, but their types are different in different branches of match (for v2 it's tx_filter::transact_acl::functions::allowed_tx_types::Decoder and for the new version it's tx_filter::transact_acl_gas_price::functions::allowed_tx_types::Decoder). Should I declare a new trait to do this? If so, wouldn't that be an overkill?

[vkomenda]

See above. I replied in a different thread by mistake.

[vkomenda]

Some repetition could be avoided in the match clauses.

I agree with @vkomenda

[varasev]

The TxPermission contract was added today.

[varasev]

Should I set the version number to 0xfffffffffffffffe here?

[vkomenda]

Should I set the version number to 0xfffffffffffffffe here?

I think so. It should be the same as the version in ethcore/src/tx_filter.rs.

[varasev]

Ok, changed: https://github.com/poanetwork/pos-contracts/blob/d5ea6319674384acbeab898bcdd26ac7207f30c7/contracts/TxPermission.sol#L64

[DemiMarie]

I fixed some of the duplication.

I fixed the duplication

[vkomenda]

Looks good!