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

Utility batch test and zero delay impl #3226

Merged
merged 15 commits into from
Jan 11, 2025
Merged

Conversation

wangminqi
Copy link
Member

We will change unstaking delay to 0.

Tests will be added for Poc that utility.batch o(f unstake and execute extrinsic) is functional in one single tx.

Copy link

linear bot commented Jan 9, 2025

@wangminqi wangminqi requested review from Kailai-Wang and a team January 9, 2025 09:05
@wangminqi
Copy link
Member Author

Since EVM user can not benefits from utility batch function, we have add conditional logic when delay = 0.
This will twist code logic a little bit, but benefits the frontend workload.

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 good

I'm just wondering why we created extra wrapper fn for some extrinsics, they seem redundant. Could you please double-check them? @wangminqi

@@ -35,6 +35,7 @@ substrate-fixed = { workspace = true }
similar-asserts = { workspace = true }
sp-core = { workspace = true, features = ["std"] }
sp-io = { workspace = true, features = ["std"] }
pallet-utility = { workspace = true, features = ["std"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact, we don't need this utility, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. I will remove this.

<CandidatePool<T>>::put(candidates);
let _ = Self::candidate_schedule_revoke(collator.clone())?;
if T::LeaveCandidatesDelay::get() == 0u32 {
Self::candidate_execute_schedule_revoke(collator)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we call the old Self::execute_leave_candidates() directly? (so we don't need to move it into another fn)

For other fn too

The origin check will pass anyway (even if it wouldn't, we could dispatch extrisnic internally too without origin check)

Copy link
Member Author

Choose a reason for hiding this comment

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

Extrinsic triggering an extrinsic may behave weird compared to an internal function.
Not sure if it is proper to do so.

@Kailai-Wang Kailai-Wang enabled auto-merge (squash) January 11, 2025 01:50
@Kailai-Wang Kailai-Wang merged commit 638b2f1 into dev Jan 11, 2025
19 checks passed
@Kailai-Wang Kailai-Wang deleted the p-1273-unstaking-cool-adjust branch January 11, 2025 08:15
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.

2 participants