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

✨ Optimize wap calculation #299

Merged
merged 1 commit into from
May 29, 2024
Merged

✨ Optimize wap calculation #299

merged 1 commit into from
May 29, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented May 16, 2024 •
edited
Loading

What?

  • Spread out the WAP on_initialize calculation into 2 blocks

Why?

  • Because the ref time was too close to the block limit

How?

  • Create a do_end_auction_closing function that does the deciding of which bids are successful, and refunds/deletes failed ones
  • do_start_community now does the WAP calculation only

Testing?

  • Run normal tests, all logic should be maintained

@JuaniRios JuaniRios mentioned this pull request May 16, 2024
Merged
@JuaniRios Graphite App
Copy link
Contributor Author

JuaniRios commented May 16, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @JuaniRios and the rest of your teammates on Graphite Graphite

@JuaniRios JuaniRios force-pushed the 05-14-restructure_funding_functions_file branch from 4a940ca to 2ad01e1 Compare May 16, 2024 13:43
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch 4 times, most recently from dcf70d9 to 9ee1d0c Compare May 17, 2024 13:49
@JuaniRios JuaniRios force-pushed the 05-14-restructure_funding_functions_file branch from 2ad01e1 to c7a42f4 Compare May 17, 2024 13:51
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch 4 times, most recently from b046766 to 3621403 Compare May 20, 2024 08:21
@JuaniRios JuaniRios changed the title optimize wap calculation ✨ Optimize wap calculation May 20, 2024
Base automatically changed from 05-14-restructure_funding_functions_file to main May 20, 2024 08:27
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch 3 times, most recently from fdb5b1b to 6d3e245 Compare May 20, 2024 10:42
@JuaniRios JuaniRios marked this pull request as ready for review May 20, 2024 11:24
@JuaniRios JuaniRios requested review from lrazovic and vstam1 May 20, 2024 11:24
@graphite-app Graphite App
Copy link

graphite-app bot commented May 20, 2024

Graphite Automations

"Auto-assign PRs to author" took an action on this PR • (05/20/24)

1 assignee was added to this PR based on Juan Ignacio Rios's automation.

@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch 3 times, most recently from 82ae855 to 7bf7a14 Compare May 20, 2024 14:06
@JuaniRios JuaniRios mentioned this pull request May 20, 2024
Closed
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch from 7bf7a14 to 6957cb0 Compare May 21, 2024 08:27
@JuaniRios JuaniRios mentioned this pull request May 21, 2024
Merged
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch from 6957cb0 to b5090b4 Compare May 21, 2024 12:47
@JuaniRios JuaniRios mentioned this pull request May 22, 2024
Merged
@JuaniRios JuaniRios mentioned this pull request May 22, 2024
Merged
lrazovic
lrazovic previously requested changes May 23, 2024
View reviewed changes
Copy link
Member

@lrazovic lrazovic left a comment

Choose a reason for hiding this comment

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

Few nits here and there, but the logic looks good! Let's merge after the approval from @vstam1 since he was more involved in the design of this.

pallets/funding/src/tests/misc.rs Outdated Show resolved Hide resolved
pallets/funding/src/weights.rs Outdated Show resolved Hide resolved
pallets/funding/src/mock.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions/3_auction.rs Show resolved Hide resolved
pallets/funding/src/functions/3_auction.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions/4_contribution.rs Outdated Show resolved Hide resolved
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch from b5090b4 to c1cec82 Compare May 24, 2024 09:42
@JuaniRios JuaniRios requested a review from lrazovic May 24, 2024 09:43
This was referenced May 24, 2024
Merged
Merged
Merged
Merged
vstam1
vstam1 requested changes May 28, 2024
View reviewed changes
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

Did we came to the conclusion that we did not have to move the rejected bids settlement outside of this function for simplicity sake for now?

I left couple of comments. Think like we said this is good enough for now with the limit of ~512 bids.

pallets/funding/src/benchmarking.rs Outdated Show resolved Hide resolved
pallets/funding/src/tests/misc.rs Outdated Show resolved Hide resolved
pallets/funding/src/types.rs Outdated Show resolved Hide resolved
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch 3 times, most recently from c40625e to fad7274 Compare May 29, 2024 11:29
@JuaniRios JuaniRios force-pushed the 05-16-optimize_wap_calculation branch from fad7274 to e317b46 Compare May 29, 2024 12:00
@JuaniRios JuaniRios requested a review from vstam1 May 29, 2024 12:02
@JuaniRios JuaniRios dismissed lrazovic’s stale review May 29, 2024 12:25

Already reviewed

@JuaniRios JuaniRios merged commit 184b645 into main May 29, 2024
@JuaniRios JuaniRios deleted the 05-16-optimize_wap_calculation branch May 29, 2024 12:26
This was referenced May 29, 2024
Merged
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstam1 vstam1 vstam1 approved these changes

@lrazovic lrazovic Awaiting requested review from lrazovic

Assignees

@JuaniRios JuaniRios

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JuaniRios @lrazovic @vstam1