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

2.1: Feature: stack-increase for PoX-2 #3282

Merged
merged 10 commits into from
Sep 21, 2022
49 changes: 49 additions & 0 deletions clarity/src/vm/database/structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,55 @@ impl<'db, 'conn> STXBalanceSnapshot<'db, 'conn> {
};
}

/// Return true iff `self` represents a snapshot that has a lock
/// created by PoX v2.
pub fn is_v2_locked(&self) -> bool {
match self.canonical_balance_repr() {
STXBalance::Unlocked { .. } => false,
STXBalance::LockedPoxOne { .. } => false,
STXBalance::LockedPoxTwo { .. } => true,
}
}

/// Increase the account's current lock to `new_total_locked`.
/// Panics if `self` was not locked by V2 PoX.
pub fn increase_lock_v2(&mut self, new_total_locked: u128) {
let unlocked = self.unlock_available_tokens_if_any();
if unlocked > 0 {
debug!("Consolidated after extend-token-lock");
}

if !self.has_locked_tokens() {
// caller needs to have checked this
panic!("FATAL: account does not have locked tokens");
}

if !self.is_v2_locked() {
// caller needs to have checked this
panic!("FATAL: account must be locked by pox-2");
}

assert!(
self.balance.amount_locked() <= new_total_locked,
"FATAL: account must lock more after `increase_lock_v2`"
);

let total_amount = self
.balance
.amount_unlocked()
.checked_add(self.balance.amount_locked())
.expect("STX balance overflowed u128");
let amount_unlocked = total_amount
.checked_sub(new_total_locked)
.expect("STX underflow: more is locked than total balance");

self.balance = STXBalance::LockedPoxTwo {
amount_unlocked,
amount_locked: new_total_locked,
unlock_height: self.balance.unlock_height(),
};
}

/// Extend this account's current lock to `unlock_burn_height`.
/// After calling, this method will set the balance to a "LockedPoxTwo" balance,
/// because this method is only invoked as a result of PoX2 interactions
Expand Down
24 changes: 16 additions & 8 deletions src/chainstate/stacks/boot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,10 +1186,6 @@ pub mod test {
lock_period: u128,
burn_ht: u64,
) -> StacksTransaction {
// (define-public (stack-stx (amount-ustx uint)
// (pox-addr (tuple (version (buff 1)) (hashbytes (buff 20))))
// (burn-height uint)
// (lock-period uint))
make_pox_contract_call(
key,
nonce,
Expand All @@ -1212,10 +1208,6 @@ pub mod test {
lock_period: u128,
burn_ht: u64,
) -> StacksTransaction {
// (define-public (stack-stx (amount-ustx uint)
// (pox-addr (tuple (version (buff 1)) (hashbytes (buff 20))))
// (burn-height uint)
// (lock-period uint))
let payload = TransactionPayload::new_contract_call(
boot_code_test_addr(),
POX_2_NAME,
Expand All @@ -1232,6 +1224,22 @@ pub mod test {
make_tx(key, nonce, 0, payload)
}

pub fn make_pox_2_increase(
key: &StacksPrivateKey,
nonce: u64,
amount: u128,
) -> StacksTransaction {
let payload = TransactionPayload::new_contract_call(
boot_code_test_addr(),
POX_2_NAME,
"stack-increase",
vec![Value::UInt(amount)],
)
.unwrap();

make_tx(key, nonce, 0, payload)
}

pub fn make_pox_2_extend(
key: &StacksPrivateKey,
nonce: u64,
Expand Down
148 changes: 138 additions & 10 deletions src/chainstate/stacks/boot/pox-2.clar
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
(define-constant ERR_INVALID_START_BURN_HEIGHT 24)
(define-constant ERR_NOT_CURRENT_STACKER 25)
(define-constant ERR_STACK_EXTEND_NOT_LOCKED 26)
(define-constant ERR_STACK_INCREASE_NOT_LOCKED 27)

;; PoX disabling threshold (a percent)
(define-constant POX_REJECTION_FRACTION u25)
Expand Down Expand Up @@ -63,8 +64,6 @@
(define-map stacking-state
{ stacker: principal }
{
;; how many uSTX locked?
amount-ustx: uint,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think you could provide a read-only function that returns the number of locked tokens for a principal, via stx-account? Asking because I'm pretty sure stacking.club depends on this information (and there could be others).

;; Description of the underlying burnchain address that will
;; receive PoX'ed tokens. Translating this into an address
;; depends on the burnchain being used. When Bitcoin is
Expand Down Expand Up @@ -560,8 +559,7 @@
;; add stacker record
(map-set stacking-state
{ stacker: tx-sender }
{ amount-ustx: amount-ustx,
pox-addr: pox-addr,
{ pox-addr: pox-addr,
reward-set-indexes: reward-set-indexes,
first-reward-cycle: first-reward-cycle,
lock-period: lock-period })
Expand Down Expand Up @@ -703,8 +701,7 @@
;; add stacker record
(map-set stacking-state
{ stacker: stacker }
{ amount-ustx: amount-ustx,
pox-addr: pox-addr,
{ pox-addr: pox-addr,
first-reward-cycle: first-reward-cycle,
reward-set-indexes: (list),
lock-period: lock-period })
Expand Down Expand Up @@ -761,6 +758,65 @@
})
)

(define-private (increase-reward-cycle-entry
(reward-cycle-index uint)
(updates (optional { first-cycle: uint, reward-cycle: uint, stacker: principal, add-amount: uint })))
(let ((data (try! updates))
(first-cycle (get first-cycle data))
(reward-cycle (get reward-cycle data)))
(if (> first-cycle reward-cycle)
;; not at first cycle to process yet
(some { first-cycle: first-cycle, reward-cycle: (+ u1 reward-cycle), stacker: (get stacker data), add-amount: (get add-amount data) })
(let ((existing-entry (unwrap-panic (map-get? reward-cycle-pox-address-list { reward-cycle: reward-cycle, index: reward-cycle-index })))
(existing-total (unwrap-panic (map-get? reward-cycle-total-stacked { reward-cycle: reward-cycle }))))
;; stacker must match
(asserts! (is-eq (get stacker existing-entry) (some (get stacker data))) none)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If (is-some (get stacker data)) is true, then wouldn't this assertion's failure be a Really Bad Thing, i.e. indicative that reward-cycle-pox-address-list was corrupted somehow, since the reward-cycle-index didn't refer to a PoX address from this stacker?

I can see why (get stacker data) could be none -- e.g. when the stacker had stacked through a delegate. But, should this function even be called when this is the case (or, can we not tell prior to calling that this is the case)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this would be a Really Bad Thing, but Clarity cannot assert a node panic, so instead, we just panic the transaction.

;; update the pox-address list
(map-set reward-cycle-pox-address-list
{ reward-cycle: reward-cycle, index: reward-cycle-index }
{ pox-addr: (get pox-addr existing-entry),
total-ustx: (+ (get total-ustx existing-entry)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can compute total-ustx in the let (it's computed twice here)

(get add-amount data)),
stacker: (some (get stacker data)) })
;; update the total
(map-set reward-cycle-total-stacked
{ reward-cycle: reward-cycle }
{ total-ustx: (+ (get total-ustx existing-total)
(get add-amount data)) })
(some { first-cycle: first-cycle,
reward-cycle: (+ u1 reward-cycle),
stacker: (get stacker data),
add-amount: (get add-amount data) })))))

(define-public (stack-increase (increase-by uint))
(let ((stacker-info (stx-account tx-sender))
(amount-stacked (get locked stacker-info))
(unlock-height (get unlock-height stacker-info))
(unlock-in-cycle (burn-height-to-reward-cycle unlock-height))
(cur-cycle (current-pox-reward-cycle))
(first-increased-cycle (+ cur-cycle u1))
(stacker-state (unwrap! (map-get? stacking-state
{ stacker: tx-sender })
(err ERR_STACK_EXTEND_NOT_LOCKED))))
(asserts! (> amount-stacked u0)
(err ERR_STACK_EXTEND_NOT_LOCKED))
(asserts! (>= increase-by u1)
(err ERR_STACKING_INVALID_AMOUNT))
(asserts! (>= (get unlocked stacker-info) increase-by)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could define amount_unlocked in the let and use that instead of (get unlocked stacker-info)

(err ERR_STACKING_INSUFFICIENT_FUNDS))
(asserts! (check-caller-allowed)
(err ERR_STACKING_PERMISSION_DENIED))
;; update reward cycle amounts
(asserts! (is-some (fold increase-reward-cycle-entry
(get reward-set-indexes stacker-state)
(some { first-cycle: first-increased-cycle,
reward-cycle: (get first-reward-cycle stacker-state),
stacker: tx-sender,
add-amount: increase-by })))
(err ERR_STACKING_UNREACHABLE))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if this (fold increase-reward-cycle-entry) is always supposed to be (some ..), then doesn't that mean that internally, tx-sender must not have stacked through a delegate? Can we check this as part of the checks above this line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, will add a check.

;; NOTE: stacking-state map is unchanged: it no longer tracks amount-stacked in PoX-2
(ok { stacker: tx-sender, total-locked: (+ amount-stacked increase-by)})))

;; Extend an active stacking lock.
;; *New in Stacks 2.1*
;; This method extends the `tx-sender`'s current lockup for an additional `extend-count`
Expand Down Expand Up @@ -811,15 +867,88 @@
;; update stacker record
(map-set stacking-state
{ stacker: tx-sender }
{ amount-ustx: amount-ustx,
pox-addr: pox-addr,
{ pox-addr: pox-addr,
reward-set-indexes: reward-set-indexes,
first-reward-cycle: cur-cycle,
lock-period: lock-period })

;; return lock-up information
(ok { stacker: tx-sender, unlock-burn-height: new-unlock-ht })))))

;; As a delegator, increase an active stacking lock, issuing a "partial commitment" for the
;; increased cycles.
;; *New in Stacks 2.1*
;; This method increases `stacker`'s current lockup and partially commits the additional
;; STX to `pox-addr`
(define-public (delegate-stack-increase
(stacker principal)
(pox-addr { version: (buff 1), hashbytes: (buff 20) })
(increase-by uint))
(let ((stacker-info (stx-account stacker))
(existing-lock (get locked stacker-info))
(available-stx (get unlocked stacker-info))
(unlock-height (get unlock-height stacker-info)))

;; must be called with positive `increase-by`
(asserts! (>= increase-by u1)
(err ERR_STACKING_INVALID_AMOUNT))

(let ((unlock-in-cycle (burn-height-to-reward-cycle unlock-height))
(cur-cycle (current-pox-reward-cycle))
(first-increase-cycle (+ cur-cycle u1))
(last-increase-cycle (- unlock-in-cycle u1))
(cycle-count (try! (if (<= first-increase-cycle last-increase-cycle)
(ok (+ u1 (- last-increase-cycle first-increase-cycle)))
(err ERR_STACKING_INVALID_LOCK_PERIOD))))
(new-total-locked (+ increase-by existing-lock))
(stacker-state
(unwrap! (map-get? stacking-state { stacker: stacker })
(err ERR_STACK_INCREASE_NOT_LOCKED))))

;; must be called directly by the tx-sender or by an allowed contract-caller
(asserts! (check-caller-allowed)
(err ERR_STACKING_PERMISSION_DENIED))

;; stacker must be currently locked
(asserts! (> existing-lock u0)
(err ERR_STACK_INCREASE_NOT_LOCKED))

;; stacker must have enough stx to lock
(asserts! (>= available-stx increase-by)
(err ERR_STACKING_INSUFFICIENT_FUNDS))

;; stacker must have delegated to the caller
(let ((delegation-info (unwrap! (get-check-delegation stacker) (err ERR_STACKING_PERMISSION_DENIED))))
;; must have delegated to tx-sender
(asserts! (is-eq (get delegated-to delegation-info) tx-sender)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these asserts would be easier to read if you moved the (get _ delegation-info) to the let

(err ERR_STACKING_PERMISSION_DENIED))
;; must have delegated enough stx
(asserts! (>= (get amount-ustx delegation-info) new-total-locked)
(err ERR_DELEGATION_TOO_MUCH_LOCKED))
;; if pox-addr is set, must be equal to pox-addr
(asserts! (match (get pox-addr delegation-info)
specified-pox-addr (is-eq pox-addr specified-pox-addr)
true)
(err ERR_DELEGATION_POX_ADDR_REQUIRED))
;; delegation must not expire before lock period
(asserts! (match (get until-burn-ht delegation-info)
until-burn-ht
(>= until-burn-ht unlock-height)
true)
(err ERR_DELEGATION_EXPIRES_DURING_LOCK)))

;; delegate stacking does minimal-can-stack-stx
(try! (minimal-can-stack-stx pox-addr new-total-locked first-increase-cycle (+ u1 (- last-increase-cycle first-increase-cycle))))

;; register the PoX address with the amount stacked via partial stacking
;; before it can be included in the reward set, this must be committed!
(add-pox-partial-stacked pox-addr first-increase-cycle cycle-count increase-by)

;; stacking-state is unchanged, so no need to update

;; return the lock-up information, so the node can actually carry out the lock.
(ok { stacker: stacker, total-locked: new-total-locked}))))

;; As a delegator, extend an active stacking lock, issuing a "partial commitment" for the
;; extended-to cycles.
;; *New in Stacks 2.1*
Expand Down Expand Up @@ -894,8 +1023,7 @@
;; update stacker record
(map-set stacking-state
{ stacker: stacker }
{ amount-ustx: amount-ustx,
pox-addr: pox-addr,
{ pox-addr: pox-addr,
reward-set-indexes: (list),
first-reward-cycle: first-extend-cycle,
lock-period: lock-period })
Expand Down
Loading