Skip to content

fix!: blocked addresses #6176

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
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix!: blocked addresses #6176

wants to merge 5 commits into from

Conversation

@rootulp
Copy link
Collaborator

@rootulp rootulp commented Nov 12, 2025

Closes #6175

Can't be merged to main until we have a v6.x branch.

@rootulp rootulp requested a review from damiannolan November 12, 2025 15:23
@rootulp rootulp self-assigned this Nov 12, 2025
@rootulp rootulp added the consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules label Nov 12, 2025
damiannolan
damiannolan previously approved these changes Nov 14, 2025
View reviewed changes
Copy link
Collaborator

@damiannolan damiannolan 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.

Great catch, did this cause an issue with these invariants?

I see that BlockedAddresses was introduced in #4400 but rather than using the maccPerms map as it should have been doing, it used the existing ModuleAccountAddrs() func incorrectly (since this does the address conversion upfront).

@damiannolan
Copy link
Collaborator

damiannolan commented Nov 14, 2025

Looks like maybe @galaparm should be added as co-author on this PR?

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 14, 2025 •
edited
Loading

Great catch, did this cause an issue with these invariants?

AFAICT it hasn't caused any real issues so this can wait until the v7.x.x release

Looks like maybe @galaparm should be added as co-author on this PR?

Can do

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 14, 2025

@galaparm if you want to be added as a co-author, I need your GitHub-verified email.

@rootulp rootulp added the WS: v7 label Nov 14, 2025
@galaparm
Copy link
Contributor

galaparm commented Nov 14, 2025

@rootulp I'd really appreciate being added as a co-author. My verified email is:
[email protected]

Thanks a lot!

@rootulp rootulp requested a review from damiannolan November 14, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@damiannolan damiannolan damiannolan approved these changes

@evan-forbes evan-forbes Awaiting requested review from evan-forbes evan-forbes is a code owner

@rach-id rach-id Awaiting requested review from rach-id rach-id is a code owner

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

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

Assignees

@rootulp rootulp

Labels

consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules WS: v7

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BlockedAddresses are incorrect

3 participants

@rootulp @damiannolan @galaparm