Skip to content

merge bitcoin#13219...#15779: benchmarks #4511

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
Merged
UdjinM6 merged 10 commits into from
Oct 25, 2021
Merged

merge bitcoin#13219...#15779: benchmarks #4511

UdjinM6 merged 10 commits into from
Oct 25, 2021

Conversation

@kwvg
Copy link
Collaborator

@kwvg kwvg commented Oct 12, 2021 •
edited
Loading

Additional Notes

  • Deviation in script used for the block assembly benchmark is due to our lack of a segregated witness implementation. The "legacy" substitute is courtesy of Bitcoin Cash.
  • For some reason BLS DKG benchmarks are borked again (at least on my system) even before this PR was being worked on but repairing them is outside the scope of the PR and therefore has not been addressed.
  • The backport for test: Add wallet_balance benchmark bitcoin/bitcoin#15779 deviates considerably from the original pull request in that
    • It is partial as it includes wallet/node separation that's not within the scope of this PR and;
    • Bech32 addresses aren't considered to be a valid CTxDestination within Dash as of now, causing an assertion failure in the middle of running benchmarks, hence the substitution with a RegTest B58 P2PKH address.

Checklist (Part 1)

Checklist (Part 2)

@kwvg kwvg marked this pull request as draft October 12, 2021 04:16
@github-actions
Copy link

github-actions bot commented Oct 13, 2021

This pull request has conflicts, please rebase.

@kwvg kwvg marked this pull request as ready for review October 25, 2021 09:28
@kwvg kwvg self-assigned this Oct 25, 2021
UdjinM6
UdjinM6 requested changes Oct 25, 2021
View reviewed changes
@kwvg kwvg requested a review from UdjinM6 October 25, 2021 15:50
UdjinM6
UdjinM6 reviewed Oct 25, 2021
View reviewed changes
UdjinM6
UdjinM6 requested changes Oct 25, 2021
View reviewed changes
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.

pls see cc25e395ed

@UdjinM6
Copy link

UdjinM6 commented Oct 25, 2021

Not sure how you do this but Gitlab fails to pick and build this branch again... local ci is here https://gitlab.com/UdjinM6/dash/-/pipelines/395050276

@kwvg
Copy link
Collaborator Author

kwvg commented Oct 25, 2021

Yeah, seems to be a random quirk, maybe it didn't go through or it isn't a fan of frequent force-pushes, I dunno but it is throwing a wrench into things. Thanks for the personal CI run!

PastaPastaPasta
PastaPastaPasta approved these changes Oct 25, 2021
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merge via merge commit

Seems correct to me, plus it's only benchmarks :D

@UdjinM6 UdjinM6 added this to the 18 milestone Oct 25, 2021
UdjinM6
UdjinM6 approved these changes Oct 25, 2021
View reviewed changes
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

@UdjinM6 UdjinM6 merged commit 61a69cf into dashpay:develop Oct 25, 2021
This was referenced Oct 31, 2021
Closed
Merged
@kwvg kwvg deleted the bench branch July 18, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

@kwvg kwvg

Labels

Bitcoin Backport

Projects

None yet

Milestone

18

Development

Successfully merging this pull request may close these issues.

3 participants

@kwvg @UdjinM6 @PastaPastaPasta