Skip to content

fix: event for manually adding pubkey to provider reg#820

Merged
shaspitz merged 5 commits intomainfrom
fix-provreg
Oct 3, 2025
Merged

fix: event for manually adding pubkey to provider reg#820
shaspitz merged 5 commits intomainfrom
fix-provreg

Conversation

@shaspitz
Copy link
Copy Markdown
Contributor

@shaspitz shaspitz commented Oct 3, 2025

overrideAddBLSKey previously didn't emit a BLSKeyAdded event, making the action not easily observable on-chain. This PR adds that event to the function, and also adds a corresponding overrideRemoveBLSKey function.

@shaspitz shaspitz changed the title fix: emit event for manually adding builder pubkey, introduce rm func fix: emit event for manually adding builder pubkey Oct 3, 2025
@shaspitz shaspitz changed the title fix: emit event for manually adding builder pubkey fix: emit event for manually adding builder pubkey to provider reg Oct 3, 2025
@shaspitz shaspitz changed the title fix: emit event for manually adding builder pubkey to provider reg fix: event for manually adding pubkey to provider reg Oct 3, 2025
@shaspitz shaspitz marked this pull request as ready for review October 3, 2025 19:10
@shaspitz shaspitz requested a review from owen-eth October 3, 2025 19:15
Comment thread contracts/contracts/core/ProviderRegistry.sol
}
}
delete blockBuilderBLSKeyToAddress[blsPublicKey];
emit BLSKeyRemoved(provider, blsPublicKey);
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.

An event shouldn't be emitted if we don't find the key in the list, could require at the beginning that blockBuilderBLSKeyToAddress[blsPublicKey] is populated

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.

Actually this could cause a different issue in the case of duplicates existing, since if we had this check then only one could be removed, and would fail on the second call. Maybe we want to consider removing all instances of the key?

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.

Other option: add the require statement, but prevent double registration in overrideAddBLSKey() so that duplicates cannot exist in the first place

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like the final idea here -> 068285c

Comment thread contracts/contracts/core/ProviderRegistry.sol
@owen-eth
Copy link
Copy Markdown
Contributor

owen-eth commented Oct 3, 2025

missing new abi generation

@shaspitz shaspitz requested a review from owen-eth October 3, 2025 20:22
Copy link
Copy Markdown
Contributor

@owen-eth owen-eth left a comment

Choose a reason for hiding this comment

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

looks good!

@shaspitz shaspitz merged commit cfb5ee0 into main Oct 3, 2025
4 of 5 checks passed
@shaspitz shaspitz deleted the fix-provreg branch October 3, 2025 21:44
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.

2 participants