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

Fix: max rate issue #4580

Merged
merged 10 commits into from
Dec 10, 2023
Merged

Fix: max rate issue #4580

merged 10 commits into from
Dec 10, 2023

Conversation

diego1q2w
Copy link
Contributor

Issue

Following the discovery in issue #4549 on the Harmony repository, there is an operational inconsistency that occurs before a block is finalized. During the process where the validator data stored in memory, are written to disk as defined in statedb.go lines 896-898, a sanity check is performed. This check executes a series of rules to ensure the integrity of the data before it's committed to disk. One particular rule, which can be found in validator.go lines 315-320, verifies that the rate is not exceeding the maximum allowed rate. If the rate is greater than the maximum, the write operation fails.

This issue became prominent after the HIP30 hard fork, which increased the minimum rate to 7%. Subsequently, validators with a 5% maximum rate were unable to adjust their fees accordingly due to the aforementioned validation rule.

In this PR, we address the issue by adjusting the max-rate to be at least equal to the sum of the minRate and the max-increase-rate. This change requires a hard fork, as it necessitates consensus across the majority of the nodes to ensure that all participants agree on the same value.

@diego1q2w
Copy link
Contributor Author

Re-opening this so the tests run

internal/params/config.go Outdated Show resolved Hide resolved
@diego1q2w diego1q2w requested a review from Frozen December 7, 2023 17:18
@diego1q2w diego1q2w merged commit 02e2fee into dev Dec 10, 2023
4 checks passed
@diego1q2w diego1q2w deleted the fix/max-rate branch December 10, 2023 18:33
MaxMustermann2 pushed a commit to MaxMustermann2/harmony that referenced this pull request Dec 15, 2023
* fix: max-rate bellow the era min-rate

* fix comments

* add localnet epoch config

* update config

* update config

* update config

* update config

* add log

* remove hip30 from localnet

* disable localnet config
ONECasey pushed a commit that referenced this pull request Dec 15, 2023
* Fix: max rate issue (#4580)

* fix: max-rate bellow the era min-rate

* fix comments

* add localnet epoch config

* update config

* update config

* update config

* update config

* add log

* remove hip30 from localnet

* disable localnet config

* engine: actually fix the 7% fee implementation

* rpc: fix transaction receipt format for eth

use the same receipt as `hmyv2_`. using a boolean variable, decide if
the addresses need to be converted to bech32. do not return a contract
address unless a contract was actually deployed in the transaction by
using a pointer address type.

* rpc: add comment indicating function is unused

with the switch to `v2.NewReceipt` for even `eth_` queries, the
`eth.NewReceipt` function is no longer used

* build: fix delegation tests

* update comment

blocks was referring to `blocks of code` and not blocks in a chain.
removed the confusing word

* rpc: remove ConvertToEth in GetBlockReceipts

* internal: max rate hard fork schedule

* internal: testnet max rate schedule

---------

Co-authored-by: Diego Nava <8563843+diego1q2w@users.noreply.github.com>
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.

3 participants