Pass transaction source to validate_transaction #5366
Conversation
| /// since it's already in the received block. Note that the custom validator | ||
| /// using either `Local` or `External` should most likely just allow `InBlock` | ||
| /// transactions as well. | ||
| InBlock, |
There was a problem hiding this comment.
A few notes about this:
Note that the custom validator..
I think what you mean here is a custom unsigned validation? now it confuses me with the term validator as in validator node.
since it's already in the received block
Am I correct to think that also when you create a block locally, once you are done with validating all the transactions, in the execution phase, you will see this InBlock? Of course all other nodes who receive the block will also see InBlock
There was a problem hiding this comment.
If I am correct, then maybe the other two variants can be called ValidationLocal and ValidationExternal since you basically only see them in the authoring's validation phase. Once you are passed this, everyone will see it as InBlock .
There was a problem hiding this comment.
We use validator in a sense of UnsignedValidator logic in that file already, but I rephrased to "custom validation logic" to make it clear.
Am I correct to think that also when you create a block locally, once you are done with validating all the transactions, in the execution phase, you will see this InBlock? Of course all other nodes who receive the block will also see InBlock
For in-block transactions we don't really run UnsginedValidator::validate at all, cause we run pre_dispatch instead. However the default pre_dispatch implementation just delegates to validate so I decided to call this case InBlock.
The only case we ever see Local/External is when we validate for transaction pool, block execution will always see only InBlock (due to pre_dispatch, not an explicity validate call).
Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-Authored-By: Nikolay Volf <nikvolf@gmail.com>
…to td-txpool-origin
tomusdrw
changed the title
|
I am pretty happy with this. The only thing that I cannot really sign off on is the dilemma regarding runtime API dilemma in the description as I don't know the exact consequences. Just a note that this also probably needs a polkadot PR. |
gavofyork
added
A8-mergeoncegreen
and removed
A0-please_review
|
Just waiting for tests to pass... |
|
Tests still failing. |
|
Still...^^ |
* Add transaction source. * Bump substrate. * Fix tests.
Addresses: #4517 (comment)
Transactions that are produced by Off-chain Workers are being flagged as "Local" transactions. This information is being passed to the runtime during
validate_transactioncall, and in turn it allows the runtime to make different decision based on where the transaction was produced.For instance we can have an unsigned transaction that will only be accepted if:
The consequence of (1) is still super important, we can't assume the transaction is "fully trusted" - it's only trusted in the same sense as inherent extrinsics are - it's just an expression of block producer opinion, we should be careful and follow the same rules as usual - i.e. avoid unbounded operations (to prevent malicious validators from DoSing the rest of the network).
I've currently CHANGED the
TaggedTransactionQueueRuntime API, but was also considering just adding a new method.The latter does not break old clients (they just don't support the feature), but makes the runtime API less nice (you have to implement two methods - or maybe we could have a default implementation that just do a fallback). It also doesn't break any existing code (like calling that API over RPC) - arguably source should not really be exposed anywhere.
I'm still unsure which version is better so open to suggestions here.