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

cmd/createcluster: fix pre-gen builder registrations #2673

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

dB2510
Copy link
Contributor

@dB2510 dB2510 commented Nov 3, 2023

This PR fixes a bug in the create cluster command that was generating incorrect builder registrations in cluster-lock.json. The issue stemmed from the usage of withdrawal addresses instead of fee recipient addresses, which resulted in signature verification failures when the command was used for the 'charon run' operation.

Here's the error log caused by this bug:
image

category: bug
ticket: #2674

Copy link

sonarcloud bot commented Nov 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e4f2047) 53.00% compared to head (e66bb4c) 53.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2673      +/-   ##
==========================================
+ Coverage   53.00%   53.03%   +0.03%     
==========================================
  Files         202      202              
  Lines       28006    28006              
==========================================
+ Hits        14844    14853       +9     
+ Misses      11301    11292       -9     
  Partials     1861     1861              
Files Coverage Δ
cmd/createcluster.go 59.82% <100.00%> (ø)

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@OisinKyne
Copy link
Contributor

OisinKyne commented Nov 3, 2023

Small feedback @dB2510

This PR fixes a critical bug in the create cluster command

I think we should develop a shared framework for how we name bug severities. This would have made mev registrations fail for a specific subset of scenarios, and local block production may have worked. Either way, I would not put 'could miss block' in the critical category here, and would save that for loss of funds not missed opportunities.

@dB2510
Copy link
Contributor Author

dB2510 commented Nov 4, 2023

Hey @OisinKyne
Yeah agreed, this is not a critical bug which can cause loss of funds. This bug was not letting charon to start as it was failing initial cluster-lock verification which is why I mentioned it as a critical bug. It can be mitigated with the --no-verify flag and then everything will work as expected even the block proposals.
Updated the PR description 👍

@dB2510 dB2510 self-assigned this Nov 6, 2023
@dB2510 dB2510 added the v0.17.2 label Nov 6, 2023
@dB2510 dB2510 linked an issue Nov 6, 2023 that may be closed by this pull request
@dB2510 dB2510 added the merge when ready Indicates bulldozer bot may merge when all checks pass label Nov 6, 2023
@obol-bulldozer obol-bulldozer bot merged commit be36a73 into main Nov 6, 2023
23 checks passed
@obol-bulldozer obol-bulldozer bot deleted the dhruv/fixcreatecluster branch November 6, 2023 07:03
@gsora gsora added v0.18.0 and removed v0.17.2 labels Nov 7, 2023
dB2510 added a commit that referenced this pull request Nov 8, 2023
This PR fixes a bug in the `create cluster` command that was generating incorrect builder registrations in cluster-lock.json. The issue stemmed from the usage of withdrawal addresses instead of fee recipient addresses, which resulted in signature verification failures when the command was used for the 'charon run' operation.

Here's the error log caused by this bug:
![image](https://github.com/ObolNetwork/charon/assets/37813203/42f3fa00-cff8-4107-b8c1-55d91f485a42)


category: bug
ticket: #2674
obol-bulldozer bot pushed a commit that referenced this pull request Nov 8, 2023
This PR cherry-picks the following PRs:
- #2685
- #2673

category: bug
ticket: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass v0.18.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect pre-gen builder registrations in create cluster ouput
4 participants