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

Adding support to send batch extrinsics to the parachain #2825

Merged

Conversation

silva-fj
Copy link
Contributor

As topic: This PR adds the structure to send batch extrinsics to the parachain asynchronously. Starting with vc_issued

Copy link

linear bot commented Jun 18, 2024

@silva-fj silva-fj requested a review from a team June 18, 2024 17:22
Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thanks! The logic looks fine, only have questions regarding the batch parameters

@Kailai-Wang Kailai-Wang requested review from felixfaisal, kziemianek and a team June 18, 2024 19:02
Copy link
Collaborator

@BillyWooo BillyWooo left a comment

Choose a reason for hiding this comment

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

In general, LGTM.
Except "too many calls", just one more point: are we considering to not use such global variables GLOBAL_PARACHAIN_EXTRINSIC_TASK in the future?
maybe @Kailai-Wang have any idea?

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

I approve it as it's in any case an improvement to current structure.

there should be a limit of byte array size going through the ocall

Please double-check it and/or change the limit

Shall we use this thread to send other extrinsics? Like teebag callbacks? My preference is yes.

Currently it's often used as:

		self.validator_accessor
			.execute_mut_on_validator(|v| v.send_extrinsics(parentchain_extrinsics))?;

Does it have the same effect as having this global thread?

@silva-fj
Copy link
Contributor Author

there should be a limit of byte array size going through the ocall

The limit for extrinsics_encoded_size is u32, it is big enough

@silva-fj
Copy link
Contributor Author

Shall we use this thread to send other extrinsics? Like teebag callbacks? My preference is yes.

Currently it's often used as:

		self.validator_accessor
			.execute_mut_on_validator(|v| v.send_extrinsics(parentchain_extrinsics))?;

Does it have the same effect as having this global thread?

So, there are a couple of differences:
When using validator_access.execute_mut_on_validator(|v| v.send_extrinsics(xts))?;, after sending the extrinsics, we seal the LightValidationState, at the moment I'm not 100% sure what this implies.

The other difference is the nature of the interval batching, if we are Ok with eventual consistency there should be no problem.

@Kailai-Wang Should I make the changes to send all extrinsics to the new thread as part of this PR? or should we create a follow up issue?

@Kailai-Wang
Copy link
Collaborator

Please do it in a separate PR : )

@silva-fj silva-fj merged commit 3c90d20 into dev Jun 25, 2024
32 of 33 checks passed
@silva-fj silva-fj deleted the p-554-implement-submitting-batched-callback-extrinsics-to-the branch June 25, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants