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

Annoyance: Transaction extension tuple can't be more than 12 elements, and tuple of tuple changes order of inherited implication and is not discoverable with metadata #6569

Open
gui1117 opened this issue Nov 21, 2024 · 0 comments · May be fixed by #6571
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@gui1117
Copy link
Contributor

gui1117 commented Nov 21, 2024

TLDR: Transaction extension tuple can't be more than 12 elements, and tuple of tuple changes order of inherited implication, and the order is not discoverable with metadata because it only contains a vector of transaction extension.

Description:

The transaction extensions are define with a tuple of transaction extension, and is part of the extrinsic that is part of a block.

pub type TxExtension = (
	frame_system::CheckNonZeroSender<Runtime>,
	frame_system::CheckSpecVersion<Runtime>,
	frame_system::CheckTxVersion<Runtime>,
	frame_system::CheckGenesis<Runtime>,
	frame_system::CheckEra<Runtime>,
	frame_system::CheckNonce<Runtime>,
	frame_system::CheckWeight<Runtime>,
	pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
	cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim<Runtime>,
	frame_metadata_hash_extension::CheckMetadataHash<Runtime>,
);

/// Unchecked extrinsic type as expected by this runtime.
pub type UncheckedExtrinsic =
	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, TxExtension>;

The block requires among other traits: Eq, PartialEq, Debug.

Those specific traits are implemented on tuple of at most 12 elements: https://doc.rust-lang.org/core/primitive.tuple.html#trait-implementations-1

To fit into 12 elements we may want to group some into tuples like:

pub type TxExtension = (
	(
		frame_system::CheckNonZeroSender<Runtime>,
		frame_system::CheckSpecVersion<Runtime>,
		frame_system::CheckTxVersion<Runtime>,
		frame_system::CheckGenesis<Runtime>,
		frame_system::CheckEra<Runtime>,
	),
	frame_system::CheckNonce<Runtime>,
	frame_system::CheckWeight<Runtime>,
	pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
	cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim<Runtime>,
	frame_metadata_hash_extension::CheckMetadataHash<Runtime>,
);

But this changes the inherited implication passed to the grouped transaction extensions:
The code is here:

let following_explicit_implications = for_tuples!( ( #( &self.Tuple ),* ) );
let following_implicit_implications = self_implicit;
for_tuples!(#(
// Implication of this pipeline element not relevant for later items, so we pop it.
let (_item, following_explicit_implications) = following_explicit_implications.pop_front();
let (item_implicit, following_implicit_implications) = following_implicit_implications.pop_front();
let (item_valid, item_val, origin) = {
let implications = (
// The first is the implications born of the fact we return the mutated
// origin.
inherited_implication,
// This is the explicitly made implication born of the fact the new origin is
// passed into the next items in this pipeline-tuple.
&following_explicit_implications,
// This is the implicitly made implication born of the fact the new origin is
// passed into the next items in this pipeline-tuple.
&following_implicit_implications,
);
Tuple.validate(origin, call, info, len, item_implicit, &implications, source)?
};
let valid = valid.combine_with(item_valid);
let val = val.push_back(item_val);
)* );

To be more precise:

  • for (A, B), C, D) the inherited implications of A is ext_version, call, C_explicit, D_explicit, C_implicit, D_implicit, B_explicit, B_implicit.
  • for (A, B, C, D) the inherited implications of A is ext_version, call, B_explicit, C_explicit, D_explicit, B_implicit, C_implicit, D_implicit.
    and for both the metadata is simply A, B, C

Solution:

  • Solution 1: Don't use tuple, make a new struct with 128 optional generics named TransactionExtensionPipeline, and derive traits on it.
    EDIT: it doesn't work well when we need to give implicit or instanciate types. But with some From implementation it can be fine.
  • Solution 2: change the order of inherited implication to always be consistent in some way: need a new method to build the implication, and then tuple will return the interleaved (A_explicit, A_implicit, B_explicit, B_implicit) for its implication. Or 2 inherited implications arguments: explicit and implicit. (call and extension version would be explicits at the end.)

cc @georgepisaltu

@gui1117 gui1117 added I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. T1-FRAME This PR/Issue is related to core FRAME, the framework. labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
1 participant