SIP-022: Emergency Fix to PoX Stacking Increase #127
Conversation
|
SIP editor review, LGTM |
| `0x07`. This ensures that miners that do not upgrade from Stacks 2.1 will not | ||
| be able to mine in Stacks 2.2. | ||
|
|
||
| * Set the peer network version bits to `0x18000007`. This ensures that follower |
There was a problem hiding this comment.
Include change for testnet?
There was a problem hiding this comment.
Good catch - I think it would be a good idea to update each network's version bits for uniformity (mocknet included).
|
SIP Editor chiming in. This looks good from my perspective and feel we should push forward ASAP. |
| this particular function in this particular contract after the Bitcoin block | ||
| activation height is treated as absent from all Stacks forks. | ||
|
|
||
| * Reset all `total-ustx` values in the `reward-cycle-pox-address-list` that |
There was a problem hiding this comment.
Is this considered a change to the chainstate specification?
Something like total-ustx of reward-cycle-pox-address-list shall not be used to calculated the locked amount.
There was a problem hiding this comment.
Can you clarify in what way this change to the on-chain data is different to auto unlock?
Can we say that this SIP requires that the solo stacker lock amounts are auto unlocked earlier than usually and immediately relocked using a new calculation of the locking amount?
I would like to better understand why this SIP is different to arbitrary on-chain data manipulation (like set the balance of hacker A to 0).
The current specification is too detailed/technical about this total ustx to understand it.
There was a problem hiding this comment.
Proposal for better understanding:
-
Audit the locked amount for solo stackers and the corresponding amounts in the table
reward-cycle-pox-address-listthat is used to determine the number of reward slots. Correct the table entry if the locked amount of the users account (derived from using stack-stx, stack-increase and stack-extend transaction) do not match the stored total amount in the tablereward-cycle-pox-address-list. The locked amount of the users' accounts shall be the correct value.This correction can be seen as an unlock at the time of activation followed by an immediate imaginary stack-stx transaction with the corrected amount.
This reset shall happen prior to any reward cycle processing in
cycle # 58 -- namely, prior to the call to
get_reward_threshold_and_participation()and prior to calculating any STX
auto-unlocks (which mutates state inreward-cycle-pox-address-list).The auditing and correction routine shall be invoked while
processing every Stacks block whose parent Stacks block was mined in a Bitcoin
block before the activation height. Given that the set of solo stackers in
cycle # 56 and cycle # 57 is small and fixed, the additional overhead of fixing all records in
reward-cycle-pox-address-listwill be minimal. Nevertheless, as a safety
precaution, each Stacks block whose parent was mined prior to the Bitcoin
block activation height must contain zero transactions and confirm zero
microblocks.
There was a problem hiding this comment.
This makes me think whether it is enough to unlock all solo stackers with a different locked amount and reward-cycle entry early and make them stack again for cycle 58
Co-authored-by: Rafael Cárdenas <[email protected]>
Co-authored-by: Rafael Cárdenas <[email protected]>
|
I already commented why altering on-chain state without any transaction is a terrible idea here: stacks-network/stacks-core#3674 (comment), so below I describe alternative approach, which in my opinion (regardless of my limited knowledge about stacks-node internals and stacking logic) should be possible to implement and deliver in a timely and safe manner. I believe you mark all transactions that interacts with Then work on PoX-3, that will replace PoX-2. Build it in such way that it will allow users to stack/delegate their tokens despite the fact they are already locked by PoX-2. |
|
Referencing for readers of this SIP. Highly likely solution: Disabling PoX for 58 and launching pox-3. See https://forum.stacks.org/t/a-bug-in-stacks-increase-call-is-impacting-stacking-rewards-this-cycle/14867/8 |
|
Hey everyone, the text of this SIP has changed substantially. In line with the public calls we've had about this, this SIP now proposes two hard forks -- one to disable PoX in cycle #58, and one to re-enable it via instantiating a fixed |
…d to rectify the problem
| * The stacking threshold is raised to account for the PoX contract's reported | ||
| increase in STX locked. | ||
|
|
||
| Furthermore, if this bug is triggered enough times, the Stacks network will crash. This is |
There was a problem hiding this comment.
Before this, add a short paragraph about stackers contributing to two reward addresses:
Similar consequences are expected when a stacker contributes to two different PoX addresses and at least one PoX address would benefit from auto-unlocking. This behaviour has been not seen in the wild.
There was a problem hiding this comment.
Similar consequences are expected when a stacker contributes to two different PoX addresses and at least one PoX address would benefit from auto-unlocking. This behaviour has been not seen in the wild.
Furthermore, if this bug is triggered enough times, the Stacks network will crash. This is
because of a runtime assertion in the PoX reward set calculation logic that
verifies that the total locked STX does not exceed the total liquid STX. This
would no longer be true due to this bug. The offending assertion is detailed
below:
like this? i don't see an issue adding this but i'd prefer it @pavitthrap gives the OK to this change
|
|
||
| * Fix the aforementioned `stacks-increase` bug. | ||
|
|
||
| * Add a `delegated-to` field in the `stacking-state` data map, so that the |
There was a problem hiding this comment.
These two delegated-to points can be summarized into one for better understanding.
- Prevent stackers from contributing to two or more PoX addresses. In particular,
- add a
delegated-tofield in thestacking-statedata map, so that thestacking-stateentry for a PoX user will indicate the principal to which
their STX are delegated (if they are using delegated stacking). - add checks to the delegated stacking public functions that validate the
delegated-tofield in the Stacker'sstacking-statemap entry.
- add a
There was a problem hiding this comment.
add a delegated-to field (with checks for the stacking public functions to validate the data) in the stacking-state data map, so that the stacking-state entry for a PoX user will indicate the principal to which their STX are delegated (if they are using delegated stacking).
?
There was a problem hiding this comment.
maybe i wasn't clear in what i wrote - what i replied with was a proposed change to address your comment.
if it looks good, i'll make the change and commit
| nodes that do not upgrade to Stacks 2.2 will not be able to talk to Stacks | ||
| 2.2 nodes. | ||
|
|
||
| The second hard fork will deactivate `pox-2` and instantiate a new PoX |
There was a problem hiding this comment.
The first hard fork will deactivate pox-2 and the second hard fork will instantiate a new PoX implementation pox-3.
There was a problem hiding this comment.
I think the wording is correct.
The first hard fork will disable pox-2, but the second hard fork will deactivate it.
There was a problem hiding this comment.
@friedger is correct. The first hard-fork will declare pox-2 to be defunct, just like pox-1 is. The second hard-fork instantiates pox-3. We're doing two hard forks because we don't have time to do one.
| there is insufficient time to execute a Stacker-based SIP vote as has been customary for | ||
| past hard forks. This SIP instead proposes that this hard fork activate at the | ||
| start of the next reward cycle (#58) at the time of this writing. | ||
|
|
There was a problem hiding this comment.
In lieu of a community vote, perhaps it would be appropriate to summon the Stack Foundation board as a stand-in.
There was a problem hiding this comment.
that's an interesting idea - would the various CAB's voting on this SIP also suffice here vs a board vote?
There was a problem hiding this comment.
- Stacks Foundation boards is only for the Foundation entity, not boards for the rest of ecosystem is how I read it.
- If we want something like that would be more appropriate to have ecosystem "decentralized boards", which is kind of what Steering Committee is acting as too? Would cross-over responsibility if that's the idea. Option is to have more members on this SC committee.
- Currently requires:
A. SIP Editor approval to "Accept",
B. CAB votes approval to "Recommend",
C. Steering Committee approval to become "Activation-in-Progress"
as per SIP-000: https://github.com/stacksgov/sips/blob/main/sips/sip-000/sip-000-stacks-improvement-proposal-process.md
| The first hard fork, which activates at the start of reward cycle #58 will disable PoX. | ||
| This hard fork would do the following: | ||
|
|
||
| * Prevent all stacking functions from being called in `pox-2`. The `pox-2` |
There was a problem hiding this comment.
Is this wording correct? I think the functions can still be called, however they will not take any effect. Maybe better wording would be:
Make all stacking functions in the
pox-2contract ineffective in affecting the locked/unlocked account balances, like thepoxcontract is now.
There was a problem hiding this comment.
i think it's correct as written , since pox will be disabled at the start of cycle 58.
changing it to the proposed may introduce confusion
There was a problem hiding this comment.
I think the current wording will cause confusion, because the functions can still be called, they just won't do anything useful. The wording here makes me think that I will get an error if I try to call them, which is not correct.
whoabuddy
added
Recommended
|
SIP Editor Update 6/30/2023: Status: Approved Full details on this SIP can be seen in detail on the SIP impact assessment sheet where RICE scoring was conducted and tracked. Additional comments also available here: |
This is a SIP for addressing the bug in
stacks-increasethat causes the PoX contract to become out-of-sync with the true number of locked STX (details here: https://forum.stacks.org/t/a-bug-in-stacks-increase-call-is-impacting-stacking-rewards-this-cycle/14867).