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

Consensus protocol upgrade to disallow proposing an empty producer schedule #6458

Closed
arhag opened this issue Dec 10, 2018 · 2 comments

Comments

Projects
None yet
1 participant
@arhag
Copy link
Contributor

commented Dec 10, 2018

Background

The privileged set_proposed_producers intrinsic allows a privilege contract, such as the system contract, to change the producer schedule. The implementation of this intrinsic does various validations on the input, however, it currently does not validate that the producer schedule is not empty.

Analysis has shown that actually calling set_proposed_producers with an empty producer schedule will not lead to any serious issues. The empty proposed schedule will eventually be promoted to pending like any other schedule, which means the block header will have a new_producers field set with an empty schedule. But due to the way the promotion logic is currently implemented, the empty pending schedule will never be promoted to active. Also, the empty pending producer schedule will not prevent a new valid proposed producer schedule from being promoted from proposed to pending. Furthermore, proposing an empty producer schedule should not cause any gaps in schedule versions for the schedules that actually make it to active. Units tests have been included (#6430) to verify these statements.

It however does mean that any clients validating block headers should accept that a valid block header can include a new_producers field which contains an empty producer schedule, but they should simply ignore this field in such an scenario: that schedule will never become active.

Consensus upgrade feature

The goal of this consensus protocol upgrade feature is to disallow proposals of empty producer schedules. Even though no real harm will be done to the blockchain if an empty producer schedule was proposed using the privileged set_proposed_producers intrinsic, it is better to disallow it in the first place.

A new consensus protocol upgrade feature will be added to trigger the changes described in this consensus upgrade proposal. The actual digest for the feature understood at the blockchain level is to be determined. For the purposes of this proposal the codename DISALLOW_EMPTY_PRODUCER_SCHEDULE will be use to stand-in for whatever the feature identifier will actually end up being.

In the privileged_api::set_proposed_producers function within wasm_interface.cpp, there should be an assertion that producers.size() > 0, but only if DISALLOW_EMPTY_PRODUCER_SCHEDULE has been activated.

Optionally, controller::set_proposed_producers can be modified to either throw an exception or return -1 if producers.size() == 0 in order to protect against any future callers of controller::set_proposed_producers, but again that change in behavior is only allowed if DISALLOW_EMPTY_PRODUCER_SCHEDULE has been activated.

@arhag arhag added the HARDFORK label Dec 10, 2018

@arhag

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

This issue depends on #6429. It will also be setup by default to be a protocol feature requiring pre-activation, thus it also depends on #6431.

@arhag

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

Resolved by #7026.

@arhag arhag closed this Mar 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.