Skip to content

🐛Fix overweight contribution#244

Merged
JuaniRios merged 9 commits into
mainfrom
fix-overweight-contribution
Apr 18, 2024
Merged

🐛Fix overweight contribution#244
JuaniRios merged 9 commits into
mainfrom
fix-overweight-contribution

Conversation

@JuaniRios

Copy link
Copy Markdown
Contributor

Built on top of #243

What?

  • The proof sizes on ProjectsToUpdate for contribution_ends_round extrinsic seems way to high. We fixed them

Why?

  • We don't know for sure

How?

  • I changed from a BoundedVec to a single tuple for ProjectsToUpdate, that seemed to fix it, but when I reran the benchmark on main it produced a similar result to the tuple change, so the original error might have been a mistake on the machine.

Anything Else?

  • Even though the change of this PR probably wasn't the cause of the fix, I think it's worth to include since the boundedVec we use is always of length 1

@JuaniRios JuaniRios self-assigned this Apr 17, 2024
@JuaniRios JuaniRios requested review from lrazovic and vstam1 April 17, 2024 14:38

@vstam1 vstam1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Probably requires Migration if we want to update Politest network.
  • Did you run all tests in pallet-funding + integration tests?
    Approved if comments are fixed

Comment thread integration-tests/src/tests/basic_comms.rs Outdated
Comment thread test_weight.rs Outdated
Comment thread test_weight.rs Outdated
Comment thread pallets/funding/src/weights.rs
@lrazovic

Copy link
Copy Markdown
Member

Also, let's fix the warnings before merging:

warning: variable does not need to be mutable
   --> pallets/funding/src/benchmarking.rs:571:7
    |
571 |         let mut block_number: BlockNumberFor<T> = inst.current_block() + T::EvaluationDuration::get() + One::one();
    |             ----^^^^^^^^^^^^
    |             |
    |             help: remove this `mut`
    |
    = note: `#[warn(unused_mut)]` on by default

warning: for loop over an `Option`. This is more readably written as an `if let` statement
    --> pallets/funding/src/lib.rs:1246:37
     |
1246 |             for (project_id, update_type) in ProjectsToUpdate::<T>::take(now) {
     |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
     |
1246 |             while let Some((project_id, update_type)) = ProjectsToUpdate::<T>::take(now) {
     |             ~~~~~~~~~~~~~~~                         ~~~
help: consider using `if let` to clear intent
     |
1246 |             if let Some((project_id, update_type)) = ProjectsToUpdate::<T>::take(now) {
     |             ~~~~~~~~~~~~                         ~~~
     ```

@JuaniRios JuaniRios merged commit 0a02d83 into main Apr 18, 2024
@JuaniRios JuaniRios deleted the fix-overweight-contribution branch April 18, 2024 09:02
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.

3 participants