-
Notifications
You must be signed in to change notification settings - Fork 41
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
add postmortem-2023-10-10 #262
Open
envestcc
wants to merge
4
commits into
iotexproject:master
Choose a base branch
from
envestcc:postmortem
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
--- | ||
Postmortem ID: 6 | ||
Title: Stake duration bonus not enabled for nft bucket | ||
Author: Chen Chen <[email protected]> | ||
Created: 2023-10-10 | ||
Updated: 2023-11-09 | ||
--- | ||
|
||
|
||
# Abstract | ||
|
||
During the testing of the Staking as NFT(IIP-13) feature on Mainnet, it was found that the vote calculation of the NFT Bucket was incorrect. This resulted in a slight decrease in the calculation of delegate total votes and the distribution of bucket rewards. To fix this issue, a hard fork upgrade is required. | ||
|
||
# Cause | ||
|
||
The votes for the NFT Bucket only calculate the staked amount and does not consider the weighting of stake duration and stake-lock. | ||
|
||
**Incorrect calculation** | ||
|
||
$$votes=amount$$ | ||
|
||
**The correct** | ||
|
||
$$votes=amount*(1+\log_{1.2}{(duration/86400)(1+m)})$$ | ||
|
||
$$m= \begin{cases} 1 & locked \\ 0 & unlocked \geq 1 \end{cases}$$ | ||
|
||
# Solution | ||
|
||
Using the accurate formula for vote calculation post hard-fork height. State patching is unnecessary as NFT votes are calculated in real-time, instead of being stored in the database. Also ensure the consistency of the vote calculation formula across blockchain, rewarding service, and web/mobile. | ||
|
||
We calculate the rewards for NFT staking from epoch 37021 to 39613 based on the new reward rules and provide users with a one-time supplemental payment for the difference. The details are shown below: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. explain 37021 is when IIP-13 contract deployed, and 39613 is when hard-fork activated |
||
|
||
| Reward Address (io) | Reward Address (eth) | Rewards (IOTX) | | ||
| ----------------------------------------- | ------------------------------------------ | -------------- | | ||
| io1q77yjjytau2t0yrs656djzu6xe63umuj9afq6j | 0x07bc49488bef14b79070d534d90b9a36751e6f92 | 226.1097975 | | ||
| io19jg5h2r5m9qfpwswdat8jzacadk5cljl9j4m98 | 0x2c914ba874d94090ba0e6f56790bb8eb6d4c7e5f | 218.281543 | | ||
| io1cdv420ksuerpgkh6wkdete7yq02ulnsp6usfdw | 0xc359553ed0e646145afa759b95e7c403d5cfce01 | 149.3072101 | | ||
| io1hwugvvl2aumf9jk64vrm98l0vglr923ntq5rfz | 0xbbb88633eaef3692cadaab07b29fef623e32aa33 | 109.3416226 | | ||
| io1ar4uhyg8l576vp2jl5wzz9wdvg73jhlglwtjad | 0xe8ebcb9107fd3da60552fd1c2115cd623d195fe8 | 81.54376056 | | ||
| io1yvvlkfcrzlkewye0dc6uaqc4gnvn5wfqkq0ctn | 0x2319fb270317ed97132f6e35ce831544d93a3920 | 42.81638365 | | ||
| io1e2rfj7mh4ejawp9dy2ns035305xyahlwmhc0c3 | 0xca86997b77ae65d704ad22a707c6917d0c4edfee | 39.67336968 | | ||
| io1x0d7ju9xmvrcj8u2g6glcpk0vqavcs5968d7j9 | 0x33dbe970a6db07891f8a4691fc06cf603acc4285 | 38.88607628 | | ||
| io17axg9h32tnjyk65vculc8u9jkegzjl4maevj84 | 0xf74c82de2a5ce44b6a8cc73f83f0b2b650297ebb | 34.01664689 | | ||
| io19s6f55ux5kuamsyfm7gn93y55tt5yn3c8tm6kj | 0x2c349a5386a5b9ddc089df9132c494a2d7424e38 | 9.705679169 | | ||
| io1cew5hztn9c5y3d26yuuwdwdl0wxpm3l42a3eta | 0xc65d4b89732e2848b55a2738e6b9bf7b8c1dc7f5 | 8.143316348 | | ||
| io19un9p5jx2r5x85m4uh8ylmphj22ee0h3myxvl3 | 0x2f2650d24650e863d375e5ce4fec3792959cbef1 | 7.258715476 | | ||
| io1e933nrdazcv55dwac6mvrn472qcnfvtvx3uuk6 | 0xc963198dbd16194a35ddc6b6c1cebe503134b16c | 6.437383934 | | ||
| io1wkflcfuupysc00a73h8590377azht4wh3x6lu5 | 0x7593fc279c092187bfbe8dcf42be3ef74575d5d7 | 6.26276931 | | ||
| io1dcuhfumkk8uznqery04t5cq6edamensask7ef8 | 0x6e3974f376b1f829832323eaba601acb7bbcce1d | 6.181853975 | | ||
| io1v7ts37jua2py826er9t3hr9tru4nne6csck2zl | 0x679708fa5cea8243ab5919571b8cab1f2b39e758 | 6.021873814 | | ||
| io1pr774gjrn8ecaug749288mrr3077eh54qfej9e | 0x08fdeaa24399f38ef11ea95473ec638bfdecde95 | 5.704900111 | | ||
| io14cntt59fx92erdm0h4x7jjmlu0lucf2umelpq2 | 0xae26b5d0a9315591b76fbd4de94b7fe3ffcc255c | 5.298415282 | | ||
| io1w4jxyy9vrlwzz0ahp2p3n2227a3hxwuna54adp | 0x75646210ac1fdc213fb70a8319a94af763733b93 | 4.904248324 | | ||
| io1q226ssx0tsw2z4uthtzcd62alq9fmm32ym72sd | 0x0295a840cf5c1ca1578bbac586e95df80a9dee2a | 3.231342886 | | ||
| io1leaukdnk42lf56eeev3l8f06g8hd0tgmep8z96 | 0xfe7bcb3676aabe9a6b39cb23f3a5fa41eed7ad1b | 2.562878204 | | ||
| io1k8rmm9jfq0p802qaa426g6gm00nmlsdr5zfa4g | 0xb1c7bd964903c277a81ded55a4691b7be7bfc1a3 | 1.841719759 | | ||
| io1jslfpm04ady9ht7vm9jl5fy930wa5t8utpwd5d | 0x943e90edf5eb485bafccd965fa24858bddda2cfc | 1.712134574 | | ||
| io16w526wlurq338h3jxr8a5gw7s7pwepq2lk8l7h | 0xd3a8ad3bfc182313de3230cfda21de8782ec840a | 0.418652519 | | ||
| io10dmh3t9uthk7w5kuc823fn96ldwg0m8wnfwy2t | 0x7b7778acbc5dede752dcc1d514ccbafb5c87ecee | 0.164680286 | | ||
| io19snrtfm6742akugdqg84rc5mcntc5e3dkupaad | 0x2c2635a77af555db710d020f51e29bc4d78a662d | 0.150885108 | | ||
| io15d0f4yzxg6ka6r4x6prdt3mflkcahlaqx7gtf6 | 0xa35e9a904646addd0ea6d046d5c769fdb1dbffa0 | 0.012317156 | | ||
| io1ftu2zrzl9gd3fxuxg806dylsaa2qjtudsafse7 | 0x4af8a10c5f2a1b149b8641dfa693f0ef54092f8d | 0.007601529 | | ||
|
||
# Impact | ||
|
||
The main impacts consist of three aspects: | ||
- On-chain data: NFT Bucket rewards are calculated incorrectly. | ||
- Partnerships: Integration progress with LSD partnership could be delayed. | ||
- Teams: Hard fork exhausts the team's human resources. | ||
|
||
# Activities | ||
|
||
- 2023-09-28 08:20 : Inconsistency found between the displayed Votes on the web and the blockchain | ||
- 2023-09-28 08:20 : After a small discussion, it was decided to align the web and reward distribution with blockchain | ||
- 2023-09-29 08:20 : Internal testing of Staking as NFT feature initiated on the mainnet | ||
- 2023-10-09 05:11 : Feedback received by the group regarding NFT staking bonus votes being zero | ||
- 2023-10-09 16:11 : After a wide-range discussion, it was decided to fix the vote counting method on blockchain and synchronize the changes to the web and rewarding service | ||
- 2023-11-06 07:59:45: Redsea hard-fork is activated | ||
|
||
# Why does this issue happen? | ||
|
||
- Why use incorrect formula | ||
- The focus was mainly on the processing logic of the NFT bucket itself, such as stake, restake-related procedures. The correctness of the vote counting logic was somewhat neglected. | ||
- It was assumed that it was equal to the amount without raising this issue. | ||
- The correct formula was not found, and the programming pattern needs to be improved. | ||
- Business background is not well understood | ||
- Why not discover during testnet | ||
- During unit testing, there was a misconception about vote counting, and the test case was incorrectly written. A formal test case review process is missing | ||
- Missing comparison of votes data during system integration testing(between chain and analystics) | ||
- Why not discover during code review | ||
- The PR was too large, making code review more challenging | ||
- The code review was not meticulous enough. | ||
|
||
# How can we avoid it in the future? | ||
|
||
- Raise any doubts you may have | ||
- Keep PRs as small as possible | ||
- Enhance Testing | ||
- Review test cases more carefully | ||
- Prepare a test plan for major upgrades | ||
- Monitor data consistency among multiple systems | ||
- Think as the perspective of user | ||
|
||
# Appendix | ||
|
||
- [staking fix weighted votes of contract staking bucket #3936](https://github.com/iotexproject/iotex-core/pull/3936) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct calculation