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.

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

Remove unused register_operator(signing_key_proof_of_ownership) argument #3264

Merged
merged 3 commits into from
Nov 27, 2024
Merged

Remove unused register_operator(signing_key_proof_of_ownership) argument #3264

merged 3 commits into from
Nov 27, 2024

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 27, 2024 •
edited
Loading

PR #3263 removed the actual check, this PR removes the unused arguments, data types, and test setup code.

Part of #3261.

Code contributor checklist:

@teor2345 teor2345 self-assigned this Nov 27, 2024
@teor2345 teor2345 added improvement it is already working, but can be better execution Subspace execution labels Nov 27, 2024
NingLin-P
NingLin-P reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@NingLin-P NingLin-P left a comment

Choose a reason for hiding this comment

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

Should also remove the OperatorSigningKey storage

crates/pallet-domains/src/benchmarking.rs Outdated Show resolved Hide resolved
@teor2345
Copy link
Contributor Author

Should also remove the OperatorSigningKey storage

I'm going to do that in another PR, because the malicious operator currently depends on that storage. So removing it is a bit complicated.

vedhavyas
vedhavyas reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@vedhavyas vedhavyas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks like you have missed the Storage that does reverse lookup of SigningKey to OperatorID

vedhavyas
vedhavyas previously approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from disable-oper-reg-sig-check to main November 27, 2024 12:20
@teor2345 teor2345 dismissed vedhavyas’s stale review November 27, 2024 12:20

The base branch was changed.

@NingLin-P NingLin-P added this pull request to the merge queue Nov 27, 2024
Merged via the queue into main with commit f4a19dd Nov 27, 2024
8 checks passed
@NingLin-P NingLin-P deleted the rem-oper-reg-sig-check branch November 27, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NingLin-P NingLin-P NingLin-P approved these changes

@vedhavyas vedhavyas vedhavyas left review comments

@nazar-pc nazar-pc Awaiting requested review from nazar-pc nazar-pc is a code owner

@rg3l3dr rg3l3dr Awaiting requested review from rg3l3dr rg3l3dr is a code owner

Assignees

@teor2345 teor2345

Labels
execution Subspace execution improvement it is already working, but can be better
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@teor2345 @vedhavyas @NingLin-P