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

[v22.1.x] backport: backport to and release 22.1.2 #6614

Open
wants to merge 7 commits into
base: v22.1.x
Choose a base branch
from

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Backports for a new version, v22.1.2

What was done?

See release notes

How Has This Been Tested?

Breaking Changes

None

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Sorry, something went wrong.

…en comparing to the default / null object; speedup CDeterministicMNList::AddMN by avoiding check to IsValid when a nullcheck is sufficient

0cf8a46 suggestions (UdjinM6)
56ac184 fmt: apply clang-format suggestions (pasta)
e18f621 fix: improper default check; add tests; use in more dml places (pasta)
ada6f2b perf: speedup of CBLSLazyPublicKey::operator== when comparing to the default / null object; speedup CDeterministicMNList::AddMN by avoiding check to IsValid when a nullcheck is sufficient (pasta)

Pull request description:

  ## Profiling Analysis
  before
  <img width="1926" alt="Pasted Graphic 11" src="https://github.com/user-attachments/assets/3d333419-eea3-4ba3-a0e1-daa1670e4d52" />

  after
  <img width="1909" alt="Pasted Graphic" src="https://github.com/user-attachments/assets/b019b62d-5033-4ed9-9fdb-34e4e1d9e76a" />

  ## Methods
  Below, is some analysis on the results of running the `protx diff` RPC 500 times. The diffs had a start block between MIN and MAX as defined below; and an end block no more than MAX_DIFF from the selected start block. We then perform some statistical analysis on the data.

  MIN_VALUE = 1500050
  MAX_VALUE = 2000050
  MAX_DIFF = 50000
  
  ## Statistical Analysis, outliers included

  ### Before
  Five-Number Summary of Execution Times:
  Min:    0.024492 sec
  Q1:     0.124626 sec
  Median: 0.243000 sec
  Q3:     0.358459 sec
  Max:    15.583948 sec

  Mean Execution Time: 0.428296 sec
  Standard Deviation: 0.933486 sec

  Linear Regression Results:
  y = 0.000001 * x + 0.308662
  R-squared: 0.008160 (Goodness of Fit)
  ![Observed Data](https://github.com/user-attachments/assets/12fdd894-0f2c-4cba-9945-5e9d084c9b24)
  ![Pasted Graphic 6](https://github.com/user-attachments/assets/97706786-73b1-417d-9036-caaa3add1290)

  ### After
  Five-Number Summary of Execution Times:
  Min:    0.038174 sec
  Q1:     0.121363 sec
  Median: 0.158175 sec
  Q3:     0.215866 sec
  Max:    16.587903 sec

  Mean Execution Time: 0.239239 sec
  Standard Deviation: 0.762387 sec

  Linear Regression Results:
  y = 0.000001 * x + 0.151169
  R-squared: 0.006918 (Goodness of Fit)
  P-value: 0.063105 (Significance)
  ![Observed Data](https://github.com/user-attachments/assets/d50e5b5b-1a67-47c9-980c-564adab083ba)
  ![Observed Data](https://github.com/user-attachments/assets/cecc0394-f484-410a-ad21-1a9bbd5a1dc7)

  ## Statistical Analysis, outliers excluded

  ### Before
  removed 76 data points
  Five-Number Summary of Execution Times (After Outlier Removal):
  Min:    0.035916 sec
  Q1:     0.211060 sec
  Median: 0.319278 sec
  Q3:     0.357963 sec
  Max:    0.572785 sec

  Mean Execution Time: 0.289764 sec
  Standard Deviation: 0.101140 sec

  Linear Regression Results (After Outlier Removal):
  y = 0.0000000199 * x + 0.286447
  R-squared: 0.000496 (Goodness of Fit)
  ![Pasted Graphic 10](https://github.com/user-attachments/assets/1f0f6389-dc8f-48a5-80b7-9b5ccb3869dc)

  ### After

  removed 32 data points
  Five-Number Summary of Execution Times (After Outlier Removal):
  Min:    0.038174 sec
  Q1:     0.119880 sec
  Median: 0.151724 sec
  Q3:     0.205017 sec
  Max:    0.355078 sec

  Mean Execution Time: 0.164165 sec
  Standard Deviation: 0.060919 sec

  Linear Regression Results (After Outlier Removal):
  y = 0.0000003119 * x + 0.111002
  R-squared: 0.399298 (Goodness of Fit)

  ![Observed Data](https://github.com/user-attachments/assets/91e97214-6afb-4c3a-a98a-cd7e5704e724)

  ## How Has This Been Tested?
  Ran unit tests locally; reindexing currently, going to let CI run functional tests

  ## Breaking Changes
  Should be none; but please think through the diff specifically related to https://github.com/dashpay/dash/compare/develop...PastaPastaPasta:dash:perf-build-simplified-mn-list-diff-bls-compare-to-null?expand=1#diff-0998f8dfc4c1089e90cbaafe9607b361035b904cd103df31e3c2339a3cbf790dR480

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    LGTM, utACK 0cf8a46

Tree-SHA512: 14eb1dd3bb85c271c1d8a381554b1d774c573336123e97cffbf3222efbbe78e201ef9f331046f8f3b9147fda13fc2ea70250a46e87adcc7da5ae3301c555eddd
…mplified mn list diff output

fab006d refactor: add var name comment (UdjinM6)
50e4004 fix: Do not assert special tx type for cbtx in simplified mn list difff output (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Fixes an edge case for debug builds, release builds aren't affected.

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes
  n/a

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  kwvg:
    utACK fab006d
  knst:
    utACK fab006d
  PastaPastaPasta:
    utACK fab006d

Tree-SHA512: ffe8439e782582eda14d10c80d628272b49fea9e39f8b95186d475b13e1a373573afa68a830639f31138e51f24b763a7fb61df1bc29eb83d3835ba8117e2d228
2f5a466 fix: resolve potential deadlock in coinjoin_tests (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  `ScanForWalletTransactions` should be called outside of `cs_wallet` lock scope which is not the case for `CTransactionBuilderTestSetup ` ctor in `coinjoin_tests.cpp` atm.

  Should fix tsan test failures like https://github.com/PastaPastaPasta/dash/actions/runs/13467587625/job/37636500963#step:8:1.

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 2f5a466; thanks for looking into it!
  kwvg:
    utACK 2f5a466

Tree-SHA512: 06a3b5d8406d6675f1a9271618dbb5b5839983b90c50c8895fce755639c8f90608748c5e5f56aecd8640420b15536c9e0b4d065b8b32eb6a2f3f731f132f1b59
…r in COPYING and debian's package

c0d3abb fix: follow-up dashpay#6546 to bump copyright year in COPYING and debian's package (Konstantin Akimov)

Pull request description:

  ## What was done?
  Bump copyright for 2025 for leftover files

  ## How Has This Been Tested?
  N/A

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK c0d3abb
  PastaPastaPasta:
    utACK c0d3abb

Tree-SHA512: 7d9e915b4a05966c72b080a388676da1b112ad9c91b17e945a4834551e71ff0b76921a20cb1c1832677d1896123e6952352d0df5865cd091d6a0afd178e6e1a3
@PastaPastaPasta PastaPastaPasta added this to the 22.1 milestone Mar 20, 2025
@UdjinM6 UdjinM6 modified the milestones: 22.1, 22.1.2 Mar 21, 2025
@UdjinM6
Copy link

UdjinM6 commented Mar 21, 2025

I think you need #6564 before doing #6586 cause otherwise it's incomplete.

@PastaPastaPasta
Copy link
Member Author

PastaPastaPasta commented Mar 21, 2025

I think you need #6564 before doing #6586 cause otherwise it's incomplete.

This shouldn't be the case. In order to "force through" the v22.1.1 release after we found this, we created a release here: https://github.com/dashpay/dash-dev-branches/commits/v22.1.1/ which only backported 6586 as we do here

Also, trying to add 6564 causes conflicts, and I guess just isn't needed

…4 LTS (`jammy`), pin QEMU to avoid segfault

20524e4 fix: pin version of QEMU to avoid segfault (Kittywhiskers Van Gogh)
c8ab705 revert: update containers and CI to use Ubuntu 24.04 LTS (`noble`) (Kittywhiskers Van Gogh)
2bcc90a revert: use default non-root user `ubuntu` introduced in Ubuntu 22.10 (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * There is a regression in `noble` that has caused a build failure in trying to package our release container ([build](https://github.com/dashpay/dash/actions/runs/13376032021)) linked to [docker/setup-qemu-action#198](docker/setup-qemu-action#198).

  * In light of this, all non-CI containers and jobs have been moved back to Ubuntu 22.04 (`jammy`) though it seems that downgrading alone may be insufficient (see [actions/runner-images#11471](actions/runner-images#11471 (comment))).
    * To remedy this we are pinning the version of QEMU as suggested in [tonistiigi/binfmt#240](tonistiigi/binfmt#240 (comment))

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 20524e4

Tree-SHA512: 26bb2cd55a0267b56f86938d97ddfa32f0cdd8a2786c0366eedbcddf706e38b6af93cd29ab98ba420cbdbd112561ded61e2dba906c4b233ad737f24730f58ddc
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 0354a14

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.

None yet

2 participants