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.

Sign up for GitHub

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

Jump to bottom

fix: post proof batching #748

Merged
merged 4 commits into from
Feb 13, 2025
Merged

fix: post proof batching #748

merged 4 commits into from
Feb 13, 2025

Conversation

cernicc
Copy link
Member

@cernicc cernicc commented Feb 13, 2025 •
edited
Loading

Description

The issue is that we can only publish 10 proofs in a single extrinsic. 1 proof = 1 partition. So in this pr we changed so that the proofs are generated only for 10 partitions at most.

This was tested by lowering the limit and checking if the prooving still works.

Checklist

  • Make sure that you described what this change does.
  • Have you tested this solution?

@cernicc cernicc linked an issue Feb 13, 2025 that may be closed by this pull request
Accommodate splitting batches in Windowed PoSt #738
Closed
@cernicc cernicc self-assigned this Feb 13, 2025
@cernicc cernicc added the ready for review Review is needed label Feb 13, 2025
th7nder
th7nder requested changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@th7nder th7nder left a comment

Choose a reason for hiding this comment

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

I agree with the solution for batching, I don't agree on the parallelization yet.

@cernicc cernicc changed the title fix(738): post proof batching fix: post proof batching Feb 13, 2025
@cernicc cernicc requested a review from th7nder February 13, 2025 13:23
@cernicc cernicc added ready for review Review is needed and removed ready for review Review is needed labels Feb 13, 2025
th7nder
th7nder previously approved these changes Feb 13, 2025
View reviewed changes
@cernicc cernicc added ready for review Review is needed and removed ready for review Review is needed labels Feb 13, 2025
th7nder
th7nder previously approved these changes Feb 13, 2025
View reviewed changes
jmg-duarte
jmg-duarte previously approved these changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

Some nits and questions, but overall looks good

@cernicc cernicc dismissed stale reviews from jmg-duarte and th7nder via af758bb February 13, 2025 17:15
@cernicc cernicc enabled auto-merge (squash) February 13, 2025 17:39
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Feb 13, 2025
@cernicc cernicc added ready for review Review is needed and removed ready for review Review is needed labels Feb 13, 2025
@cernicc cernicc merged commit 3799cfe into develop Feb 13, 2025
7 of 12 checks passed
@cernicc cernicc deleted the fix/738/splitting_batches_post branch February 13, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@th7nder th7nder th7nder approved these changes

@jmg-duarte jmg-duarte jmg-duarte approved these changes

@Jinxit Jinxit Awaiting requested review from Jinxit

@neriumpete neriumpete Awaiting requested review from neriumpete

@aidan46 aidan46 Awaiting requested review from aidan46

Assignees

@cernicc cernicc

Labels
ready for review Review is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accommodate splitting batches in Windowed PoSt
3 participants
@cernicc @th7nder @jmg-duarte