Feature request for AuRa: Extended service transactions

[afck]

Validators need to be able to make service transactions with zero gas price. There is already a service transaction checker feature that allows configuring a contract to whitelist nodes for this. In addition, a transaction permissions contract can decide which types of transactions a node is allowed to make: transfer, contract deployment and/or contract interaction. The ABIs for these are service_transaction.json and tx_acl.json. They seem to be used in ServiceTransactionChecker and TransactionFilter.

However, these two features are separate and the permissions are pretty coarse-grained. We'd like to whitelist

• the current validators (→ sender address)
• to call (→ transaction type)
• specific contracts (→ receiver address)
• in service transactions (→ gas price zero).

Let's add an extended version of allowedTxTypes (suggested by @varasev):

function allowedTxTypesExtended(address sender, address receiver, uint256 gasPrice) public view returns (uint32)

That would allow writing a transaction permissions contract taking all of the above into account.

We need to figure out:

• whether we also need to extend the service transaction checker ABI, or whether that's redundant now,
• whether we should extend tx_acl.json or add a separate, new type of contract,
• whether and how to use the registry contract with it.

[varasev]

whether we also need to extend the service transaction checker ABI, or whether that's redundant now,

No, we don't need to extend it. The current interface and implementation of Certifier contract fits our current needs.

whether we should extend tx_acl.json or add a separate, new type of contract

I propose just to add separate allowedTxTypesExtended function.

whether and how to use the Registry contract with it.

I think there is no need in changing of Parity code for this. Parity uses the Registry contract as described here: https://wiki.parity.io/Permissioning.html#how-it-works-3

[afck]

I agree, if we just extend the existing permissions contract, we don't need to change the way the registry contract works.

[afck]

Then I think all we have to do for the implementation is adding a version 3 branch here where we call the new function instead, and extending the JSON ABI.

[vkomenda]

Do we need a copy of the transact_contract function with the hardcoded gas_price: 0? Or would contracts with 0 gas price be called completely differently?

[afck]

Yes, that's what I'm thinking, too. I added something like that to #52. If you're okay with it, feel free to merge that PR.

[vkomenda]

OK, thanks. That works for #50 as well. I'll use it.

[phahulin]

If we add a new version for the transactionPermissionContract, we can use the same function name allowedTxTypes with new signature.
I think this way is better than extending existing contract with a new function allowedTxTypesExtended because it lets us avoid edge cases like allowedTxTypes(sender, to, value) == true, but allowedTxTypesExtended(sender, to, value, gasPrice) == false

[DemiMarie]

I will take this one.

[phahulin]

@DemiMarie I also started it here

[afck]

Looks like this has been extended before. The discussion here is probably relevant: openethereum#8400

[varasev]

Yes, it looks like so. The current tx_acl.json contains new ABI: https://github.com/paritytech/parity-ethereum/blob/master/ethcore/res/contracts/tx_acl.json

The example of the contract:
https://gist.github.com/VladLupashevskyi/84f18eabb1e4afadf572cf92af3e7e7f

They still didn't update the docs for some reason: https://wiki.parity.io/Permissioning.html#how-it-works-1

Anyway, we still need gasPrice parameter as well.

[DemiMarie]

Do we really need a gasPrice parameter? It seems to me that it will always be zero.

[afck]

Do we really need a gasPrice parameter? It seems to me that it will always be zero.

For our purposes we're just adding it to distinguish service transactions (gas price 0) from regular transactions. That way we'll be able to allow validators to make both

• service transactions to specific contracts and
• regular transactions in any way they like.

[varasev]

Do we really need a gasPrice parameter? It seems to me that it will always be zero.

Yes, we do. The gasPrice will be zero for those transactions which are made by validator with Parity engine (when calling reportMalicious, commitHash, or revealSecret function; according to the issues #50 and #51). In other cases the validator will have to set transaction's gas price to the min possible value for a network (e.g., 1 gwei).

The to and gasPrice parameters together in allowedTxTypes function let us control whether the validator's transaction should be approved or declined depending on destination address of transaction and its gas price.

For example, if the validator sends a few coins to some address, he must set gas price to 1 gwei. If he set gas price to zero, such a transaction must be declined by allowedTxTypes.

On the other hand, if the validator calls some function of ValidatorSet or Random contract with any gas price (even zero), the allowedTxTypes function should approve such a transaction.

That's why we need the gasPrice parameter in allowedTxTypes.

To avoid DDoS with zero gas price for our contracts, we will use tx.gasprice property inside the contracts. All state-changing functions of our contracts (except those three funcs) will require tx.gasprice to be at least min allowed gas price.

[varasev]

The Certifier and TxPermission contracts were added.

[DemiMarie]

Fixed by #60