Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Operational Transaction. #3106

Closed
wants to merge 4 commits into from
Closed

Operational Transaction. #3106

wants to merge 4 commits into from

Conversation

kianenigma
Copy link
Contributor

closes #3092

TODOs:

  • Now that the Weighable is actually doing more than merely communicating weight info up to the rest of the stack, it should probably be renamed to sth more general. Likewise the #[weight] thingy.

By definition, it can be applied to anything which lives in decl_module, it is static information (stateless except for input parameters it receives) and it describes the dispatch function.#[meta =$x] and pub trait DispatchMetaInfo?

@kianenigma kianenigma added the A0-please_review Pull request needs code review. label Jul 11, 2019
@kianenigma kianenigma changed the title Operational tx weight. Operational Transaction. Jul 11, 2019
fn priority(&self, len: usize) -> TransactionPriority {
self.function.priority(len)
}
fn is_block_full(&self, block_weight: Weight, len: usize) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay this is not a great name since it is misleading. Should be something along the lines of xt.will_cause_full_block() -> bool

Copy link
Contributor

@Demi-Marie Demi-Marie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime is not allowed to panic.

fn priority(&self, _len: usize) -> $crate::dispatch::TransactionPriority {
match self {
$( $call_type::$fn_name(..) => $crate::dispatch::Weighable::priority(&$weight, _len), )*
$call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ensured at compile-time. For example, __PhantomItem could contain Infallible, and then the unreachable! could be replaced by a pattern-match against it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkchr this pattern can be replaced probably everywhere in storage macros? I borrowed the unreachable!() from other places in the code.

_block_weight,
_len,
), )*
$call_type::__PhantomItem(_, _) => { unreachable!("__PhantomItem should never be used.") },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


fn is_block_full(&self, block_weight: Weight, len: usize) -> bool {
match self {
TransactionWeight::Basic(_, _) => self.weight(len) + block_weight > MAX_TRANSACTIONS_WEIGHT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TransactionWeight::Basic(_, _) => self.weight(len) + block_weight > MAX_TRANSACTIONS_WEIGHT,
TransactionWeight::Basic(_, _) => self.weight(len).checked_add(block_weight).map(|weight| weight > MAX_TRANSACTIONS_WEIGHT).unwrap_or(true),

@@ -70,7 +70,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 109,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
spec_version: 109,
spec_version: 111,

// Operational tx.
assert_eq!(Call::<TraitImpl>::security_tx().weight(100), 0);
assert_eq!(Call::<TraitImpl>::security_tx().priority(100), u64::max_value());
// one simply does not return false of the transaction type is operational!.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// one simply does not return false of the transaction type is operational!.
// Operational transactions are never denied no matter how full
// the block is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough. I am personally pretty much okay with having some humor/informal statements as long as it is in test code :p

@kianenigma kianenigma added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. B2-breaksapi labels Jul 15, 2019
@kianenigma kianenigma closed this Jul 16, 2019
@kianenigma kianenigma deleted the operational-tx branch August 23, 2019 12:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-in_progress Pull request is in progress. No review needed at this stage.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operational Transactions
3 participants