Unsigned Validation best practices#5563
Conversation
| /// | ||
| /// To avoid conflicts between different pallets it's recommended to build `requires` | ||
| /// and `provides` tags with a unique prefix. | ||
| pub fn for_pallet(prefix: &'static str) -> ValidTransactionBuilder { |
There was a problem hiding this comment.
I wouldn't call this for pallet but rather with_prefix. It is better to demonstrate exactly what is happening.
also pallet is.. pallet and primitives should not be pallet oriented.
There was a problem hiding this comment.
I wouldn't call this for pallet but rather with_prefix. It is better to demonstrate exactly what is happening.
I'd argue that it's less clear. What prefix is that? What does it apply to?
also pallet is.. pallet and primitives should not be pallet oriented.
That's arguable as well. I feel that the frame and sp-runtime distinction is quite lost already many things are quite frame specific. Although it's a pretty lame argument, since it doesn't justify adding even more frame-specific things.
That said maybe with_tags_prefix would be a good common ground, wdyt?
| /// | ||
| /// The tag will be encoded and prefixed with pallet prefix (if any). | ||
| /// If you'd rather add a raw `require` tag, consider using `#combine_with` method. | ||
| pub fn and_requires(mut self, tag: impl Encode) -> Self { |
There was a problem hiding this comment.
won't this get corrupt if I call and_requires(a).and_requires(b) instead if and_provides((a, b).encode())? they name kinda suggests that you can call and_provides multiple times.
Either way I think the implementation should be safeguarded against it.
same for and_provides .
There was a problem hiding this comment.
and by corrupt I only mean that you push the prefix multiple times.
There was a problem hiding this comment.
Not sure I follow. and_requires/and_provides append a single Tag. All tags will be prefixed. Calling it twice is perfectly fine, but it's definitely not the same as calling once with a tuple.
There was a problem hiding this comment.
yeah my understanding was that PREFIX is added only once. But I was wrong.
|
Working on companion PR just now, will submit shortly. |
4 participants
Introducing:
ValidTransactionbuilder to take care of prefixingprovidestags.UnsignedPriorityies to make sure the runtime has a way to control inter-pallet priorities of unsigned transactions.Both should be considered best practices when implementing
UnsignedValidator, and the builder should make it easier to follow them.