-
Notifications
You must be signed in to change notification settings - Fork 186
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
Recipe for a runtime without FRAME #144
base: master
Are you sure you want to change the base?
Conversation
…-hack got cut short.
* bump dependencies * update transaction_pool api
I've pushed a few more commits trying to solve this, and adding debugging output. @bkchr I've added the transaction handling logic to @gnunicorn I've added lots of debugging output regarding the state root. I'm not sure what else to do. It seems the error, Here is the output of creating an empty block using manual seal.
Some questions / mysteries:
|
Verify, | ||
} | ||
}; | ||
//TODO this is a weird import. Is this correct? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That is normally done by the macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why do you need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need it in order to call Self::finalize_block()
from with execute_block
.
After the last two commits, the state root now matches. This is good. But the node still crashes after one block. It seems the block is hashing differently during authoring than it is later on. Logs from basic-pow node
Logs from manual-seal node
|
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Ok(ValidTransaction{ | ||
priority: 1u64, | ||
requires: Vec::new(), | ||
// This hach was necessary to make the node accept any transactions at all | ||
// When I was setting provides to an empty vec, submiting a transaction failed | ||
// the RPC responded {"code":-32603,"message":"Unknown error occurred","data":"Pool(NoTagsProvided)"} | ||
// Adding this provides tag solved that. Solutions moving forward: | ||
// 1. Require a nonce with each transaction | ||
// 2. Try to relax the TxPool's requirement that every transaction provide some tag | ||
provides: vec![vec![0]], | ||
longevity: TransactionLongevity::max_value(), | ||
propagate: true, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@todr The intention of this simple runtime is that transactions can be applied in absolutely any order. There are no dependencies between transactions and no meaningful tags to associate with the transactions. But https://github.com/paritytech/substrate/blob/75113aae02ee3cc265aaaa13cdf5a79c7ff98105/client/transaction-pool/graph/src/pool.rs#L410-L412 enforces that each transaction provide at least one tag. Is there any harm in removing this requirement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops... @tomusdrw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, somehow missed the previous mention.
The dependencies are indicated via requires
tag set, provides
guarantees de-duplication of the transactions.
Imagine that we allow no provides
tag. The same transaction can then be included in the pool and in the block as many times as you would like - so each time you receive the transaction over network from your peers it would be imported to the pool and added to the block, so you would end up with infinite number of copies of the same transaction.
I'd suggest adding provides: vec![hash(tx.encode())]
here. This will de-duplicate transactions floating around the network, but note that it doesn't prevent replaying the same transaction in consecutive blocks.
The runtime must include some kind of replay protection and at some point return Stale
from validate_transaction
for transactions that have already been applied in the network.
I'd suggest adding at least apply_before_block
parameter to each extrinsic, so that we can detect stale transactions. Note that it still does not prevent replaying the same transaction over and over again (in the same block or subsequent blocks) when it's valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A scheme I had in mind to allow both sequential and parallel transactions some time ago looks like this.
- Each transaction has
(random_salt, nonce)
. - If you want transactions to be included in parallel you set
nonce=0
and choose a randomsalt
. - If you want transactions to be included sequentially you use the same
salt
and increment thenonce
. - Runtime maintains storage entries for:
nonce_expirations: DoubleMap<BlockNumber, Salt, ()>,
next_nonce: Map<Salt, (Nonce, BlockNumber)>,
Salts are valid for fixed X
blocks, and every time a salt is used we do something like this:
let expire_at = System::current_block_number() + X;
// Update next expected nonce and pruning block.
let (nonce, block) = next_nonce.get(salt).unwrap_or_else(|| (0, 0));
next_nonce.insert(salt, (nonce + 1, expire_at));
// remove previous expiration notice
nonce_expirations.remove((block, salt));
// insert new expiration
nonce_expirations.insert((expire_at, salt), ());
And then the runtime can use next_nonce
to figure out validity of the transaction, and in on_initialize
we take all nonce_expirations
at that particular block and prune out all the salts (since we use DoubleMap
we can get all entries efficiently).
Thanks to Basti's last suggestion we now have block production working. I'm now trying to submit extrinsics. Our extrinsic type is an enum with three variants. According to the scale docs the variants should encode as Submitting
But there are two oddities. The first oddity is that submitting
The second oddity is that when I submit the apparently working |
/// Opaque block header type. | ||
pub type Header = generic::Header<BlockNumber, BlakeTwo256>; | ||
/// Opaque block type. | ||
pub type Block = generic::Block<Header, OpaqueExtrinsic>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoshOrndorff you are using here the OpaqueExtrinsic
and that works a little bit different on the node side. AFAIK it has the length prepended, which you not do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So OpaqueExtrinsic
is just an alias for Vec<u8>
. And scale docs for Vec<_>
do indeed say the encoding is "prefixed with a compact encoding of the number of items, followed by each item's encoding concatenated in turn".
I will be encoding a single byte which means I should prepend with the compact encoding for the number one which is 0x04
according to the same scale docs (in addition to doing the encoding myself, I notices that the number 1 is an example given.
Trying to encode my transaction this way I get the following output
$ curl http://localhost:9933 -H "Content-Type:application/json;charset=utf-8" -d '{
"jsonrpc":"2.0",
"id":1,
"method":"author_submitExtrinsic",
"params": ["0x0400"]
}'
{"jsonrpc":"2.0","error":{"code":1002,"message":"Verification Error: Execution(ApiError(\"Could not convert parameter `tx` between node and runtime: No such variant in enum FramelessTransaction\"))","data":"RuntimeApi(\"Execution(ApiError(\\\"Could not convert parameter `tx` between node and runtime: No such variant in enum FramelessTransaction\\\"))\")"},"id":1}
Although the error message is long, I think this is actually a step in the right direction. It is now clearly trying to decode into FramelessTransaction
which is exactly what I want. So the remaining mysteries are:
- Now that I know to prepend the length, how do I actually encode the
FramelessTransaction
variant? - Why did submitting
0x00
ever even partially work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Length of 1 prepended and 1 byte added for your enum variant of the tx.
-
It is an empty transaction which probably failed in the runtime than.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Length of 1 prepended and 1 byte added for your enum variant of the tx.
I believe that's what I've done. Length of 1 encodes as 0x04
(citation). My enum variant is 0x00
.
That transaction fails with the error Verification Error: Execution(ApiError(\"Could not convert parameter
tx between node and runtime: No such variant in enum FramelessTransaction\"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you had 0x0400
send to the node? (Not sure if that hex is in correct order)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, submitting 0x0400
is the example quoted above.
I've also tried submitting 0x0004
(which I think is backwards) and it behaves the same as 0x00
. This makes sense because the length prefix is 0 so the next byte is ignored.
I'm continuing this work in https://github.com/substrate-developer-hub/substrate-node-template/pull/347. It looks like it's actually processing transactions now! |
This PR addresses #131 by demonstrating a runtime that does not use Frame. The runtime is as minimal as reasonable and has a single boolean storage value. Transaction can either set, clear, or toggle this storage value. Transaction are not signed.