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

Do only the necessary validation prior to including transaction into a chunk #84

Open
lexfrl opened this issue Jun 12, 2020 · 7 comments

Comments

@lexfrl
Copy link

lexfrl commented Jun 12, 2020

Rationale

Currently, the network layer is aware of receipt (transaction) actions semantics in a way that node does TX validation outside of a chunk validation:
https://github.com/nearprotocol/nearcore/blob/a3ffe63f03c6d0e5f121aa783ff1091428036802/runtime/runtime/src/verifier.rs#L57-L178

On a chunk validation, runtime does the same TX validation once again. This proposal suggest a change to the protocol to allow to make only the necessary TX checks prior to the inclusion into a chunk.

Solution

We can change SignedTransaction to the following, exposing only the necessary information to check that the signature is authentic, the signer (access_key) is solvent to cover attached_gas and attached_gas > config.receipt_cost_per_byte * receipt.len():

SignedTransaction {
    signer: AccountId,
    public_key: PublicKey,
    signature: Singnature,
/// gas attached to buy calculated on a client, according to the cost of a receipt
/// `config.receipt_cost_per_byte * receipt.len()` + `(total_cost of actions)`
    attached_gas: Gas,
    gas_price: Balance,
    receipt: Vec<u8>
}

We can have a receipt_cost_per_byte in the chain config and thus (on the chunk generation) we have to check only that account (access_key) has funds to cover gas to parse ActionReceipt. Thus client do not aware of receipt structure at all.

This way, the minimum attached_gas to be included into a chunk will be equal to config.receipt_cost_per_byte * transaction.receipt.len(). Of course, signer (nearlib) better attach gas to cover total cost of included actions otherwise transaction outcome will be ExecutionOutcome(ExecutionStatus::Failure) on a chunk application (the validator will burn all attached_gas for the actual CPU work done).

Recap

Proposed change allows chunk producer do not parse and validate Tx body (actions) prior including it into a chunk and leave this validation to validator. This gives us the following interdependent advantages:

  • it speedups chunk production (since no need to execute the same validation logic twice);
  • it abstracts networking layer (chain) makes it independent from a state transition function (runtime) semantics;
  • node does minimum checks to validate transaction;
  • makes transaction deserialization and (semantic) validation costs accounted (included into a fee), which reduces a node DDoS attack surface

Issues:

  • nearlib changes required: it must calculate attached_gas, based on the actions, included in transaction

Comments?

@evgenykuzyakov @nearmax @bowenwang1996 @ilblackdragon @SkidanovAlex

@bowenwang1996
Copy link
Collaborator

I don't understand why this is needed. Which part of the validation is unnecessary today?

@evgenykuzyakov
Copy link
Contributor

The reason for the validation before including into the chunk is to only include the valid transactions in the chunk. It decreases the size of the chunk by putting extra effort on the chunk producer, but decreases the load from everybody else who's validating this chunk.

@lexfrl
Copy link
Author

lexfrl commented Jun 12, 2020

It decreases the size of the chunk by putting extra effort on the chunk producer, but decreases the load from everybody else who's validating this chunk.

Size of the chunk can be measured by its size in bytes and CPU time to execute on signer. The idea is to split it and make sure that sender paid at least for transmission. During transaction application, we charge account incrementally action-by-action (like we do it when we create Promises).

In general idea is to make user input opaque to the chain layer and deserialize actions only on a chunk application. So, in a trust-less setting it makes sense to execute user input in accountable environment.

@lexfrl
Copy link
Author

lexfrl commented Jun 12, 2020

The reason for the validation before including into the chunk is to only include the valid transactions in the chunk.

It fact, the proposal simply changes the definition of a "valid transaction", making it more general.

@bowenwang1996
Copy link
Collaborator

@fckt the transaction itself is usually quite cheap. This will allow people to spam with invalid transactions at a relatively low cost.

@lexfrl
Copy link
Author

lexfrl commented Jun 12, 2020

@bowenwang1996
The problem is that right now people can spam block producer with invalid transactions even for free, since this operation (even if we suppose that it's a cheap operation, but still O(2* n) - deserialization + validation) is not accounted.

The idea is to allow (potentially invalid from a state transition function stand point) transactions to join chunk, since account is guaranteed to be charged at least by amount of config.receipt_cost_per_byte * receipt.len() - this means chunk producer guarantees that account will (at least) compensate transaction validation process and will not allow to spam itself with invalid input at all. Since the cost of input deserializing and interpretation is guarantied to be covered, we can allow runtime (state transition function) to interpret user input in a way it want to (as long as interpretation has a linear asymptotic complexity against input).

@evgenykuzyakov
Copy link
Contributor

But you still need to pull account and access_key, verify signature and make sure there is enough cost. Then you'll need to pre-charge the account/access key for TX costs first before you start validate transactions.
It doesn't address the DDOS issue at least if the receipt is part of the transaction, because you still need to receive a large transaction, hash it and verify account/access key. The rest of validation right now if fairly cheap. Probably O(n)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants