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

PoX-4 Signing Keys Stacking-State #4092

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion stackslib/src/chainstate/nakamoto/coordinator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use stacks_common::types::chainstate::{
};
use stacks_common::types::{Address, StacksEpoch};
use stacks_common::util::vrf::VRFProof;
use wsts::curve::point::Point;
use wsts::curve::point::{Compressed, Point};

use crate::chainstate::burn::db::sortdb::{SortitionDB, SortitionHandle};
use crate::chainstate::burn::operations::BlockstackOperationType;
Expand All @@ -50,6 +50,11 @@ use crate::net::test::{TestPeer, TestPeerConfig};
fn advance_to_nakamoto(peer: &mut TestPeer) {
let mut peer_nonce = 0;
let private_key = peer.config.private_key.clone();
let public_key = StacksPublicKey::from_private(&private_key);
let compressed_key = Compressed::try_from(public_key.to_bytes_compressed().as_slice())
.expect("Failed to convert public key to compressed struct");
let public_key_point =
Point::try_from(&compressed_key).expect("Failed to convert public key to point");
let addr = StacksAddress::from_public_keys(
C32_ADDRESS_VERSION_TESTNET_SINGLESIG,
&AddressHashMode::SerializeP2PKH,
Expand All @@ -69,6 +74,7 @@ fn advance_to_nakamoto(peer: &mut TestPeer) {
PoxAddress::from_legacy(AddressHashMode::SerializeP2PKH, addr.bytes.clone()),
12,
34,
&public_key_point,
);
vec![stack_tx]
} else {
Expand Down
27 changes: 27 additions & 0 deletions stackslib/src/chainstate/stacks/boot/contract_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ lazy_static! {
boot_code_id("pox-2", false);
pub static ref COST_VOTING_CONTRACT_TESTNET: QualifiedContractIdentifier =
boot_code_id("cost-voting", false);
pub static ref POX_4_CONTRACT_TESTNET: QualifiedContractIdentifier =
boot_code_id("pox-4", false);
pub static ref USER_KEYS: Vec<StacksPrivateKey> =
(0..50).map(|_| StacksPrivateKey::new()).collect();
pub static ref POX_ADDRS: Vec<Value> = (0..50u64)
Expand Down Expand Up @@ -559,6 +561,31 @@ impl HeadersDB for TestSimHeadersDB {
}
}

#[test]
fn pox_4_instantiates() {
let mut sim = ClarityTestSim::new();
sim.epoch_bounds = vec![0, 1, 2];
let delegator = StacksPrivateKey::new();

let expected_unlock_height = POX_TESTNET_CYCLE_LENGTH * 4;

// execute past 2.1 epoch initialization
jcnelson marked this conversation as resolved.
Show resolved Hide resolved
sim.execute_next_block(|_env| {});
sim.execute_next_block(|_env| {});
sim.execute_next_block(|_env| {});

sim.execute_next_block(|env| {
env.initialize_versioned_contract(
POX_4_CONTRACT_TESTNET.clone(),
ClarityVersion::Clarity2,
&boot::POX_4_TESTNET_CODE,
None,
ASTRules::PrecheckSize,
)
.unwrap()
});
}

#[test]
fn pox_2_contract_caller_units() {
let mut sim = ClarityTestSim::new();
Expand Down
15 changes: 9 additions & 6 deletions stackslib/src/chainstate/stacks/boot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1664,12 +1664,8 @@ pub mod test {
addr: PoxAddress,
lock_period: u128,
burn_ht: u64,
public_key: &Point,
) -> StacksTransaction {
// ;; TODO: add signer key
// (define-public (stack-stx (amount-ustx uint)
// (pox-addr (tuple (version (buff 1)) (hashbytes (buff 32))))
// (burn-height uint)
// (lock-period uint))
let addr_tuple = Value::Tuple(addr.as_clarity_tuple().unwrap());
let payload = TransactionPayload::new_contract_call(
boot_code_test_addr(),
Expand All @@ -1680,6 +1676,8 @@ pub mod test {
addr_tuple,
Value::UInt(burn_ht as u128),
Value::UInt(lock_period),
Value::buff_from(public_key.compress().data.to_vec())
.expect("Failed to serialize aggregate public key"),
],
)
.unwrap();
Expand Down Expand Up @@ -1792,13 +1790,18 @@ pub mod test {
nonce: u64,
addr: PoxAddress,
lock_period: u128,
signing_key: Option<&Point>,
) -> StacksTransaction {
let signing_key = Value::Optional(OptionalData {
data: signing_key
.map(|key| Box::new(Value::buff_from(key.compress().data.to_vec()).unwrap())),
});
let addr_tuple = Value::Tuple(addr.as_clarity_tuple().unwrap());
let payload = TransactionPayload::new_contract_call(
boot_code_test_addr(),
POX_4_NAME,
"stack-extend",
vec![Value::UInt(lock_period), addr_tuple],
vec![Value::UInt(lock_period), addr_tuple, signing_key],
)
.unwrap();

Expand Down
47 changes: 30 additions & 17 deletions stackslib/src/chainstate/stacks/boot/pox-4.clar
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,20 @@
(define-constant ERR_DELEGATION_WRONG_REWARD_SLOT 29)
(define-constant ERR_STACKING_IS_DELEGATED 30)
(define-constant ERR_STACKING_NOT_DELEGATED 31)
(define-constant ERR_STACKING_DURING_PREPARE_PHASE 32)
(define-constant ERR_STACK_EXTEND_NOT_SIGNED 33)

;; PoX disabling threshold (a percent)
(define-constant POX_REJECTION_FRACTION u25)

;; Valid values for burnchain address versions.
;; These first four correspond to address hash modes in Stacks 2.1,
;; and are defined in pox-mainnet.clar and pox-testnet.clar (so they
;; cannot be defined here again).
;; (define-constant ADDRESS_VERSION_P2PKH 0x00)
Copy link
Member

Choose a reason for hiding this comment

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

If you're now using is-in-mainnet instead of a separate pox-4-mainnet.clar, then where are these constants defined now?

;; (define-constant ADDRESS_VERSION_P2SH 0x01)
;; (define-constant ADDRESS_VERSION_P2WPKH 0x02)
;; (define-constant ADDRESS_VERSION_P2WSH 0x03)
(define-constant ADDRESS_VERSION_NATIVE_P2WPKH 0x04)
(define-constant ADDRESS_VERSION_NATIVE_P2WSH 0x05)
(define-constant ADDRESS_VERSION_NATIVE_P2TR 0x06)
;; (define-constant ADDRESS_VERSION_NATIVE_P2WPKH 0x04)
Copy link
Member

Choose a reason for hiding this comment

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

Please don't commit commented-out code

;; (define-constant ADDRESS_VERSION_NATIVE_P2WSH 0x05)
;; (define-constant ADDRESS_VERSION_NATIVE_P2TR 0x06)
;; Keep these constants in lock-step with the address version buffs above
;; Maximum value of an address version as a uint
(define-constant MAX_ADDRESS_VERSION u6)
Expand Down Expand Up @@ -117,7 +116,9 @@
;; previous stack-stx calls, or prior to an extend)
reward-set-indexes: (list 12 uint),
;; principal of the delegate, if stacker has delegated
delegated-to: (optional principal)
delegated-to: (optional principal),
;; signing key for Nakamoto, only 'none' when delegated & before delegate calls stack-aggregation-commit-indexed
Copy link
Member

@jcnelson jcnelson Dec 13, 2023

Choose a reason for hiding this comment

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

I don't think this is ever none if we're setting the signing key on delegate-stack-stx (which IIRC is the plan now)

EDIT: this comment is out of date

signing-key: (buff 33)
}
)

Expand Down Expand Up @@ -576,13 +577,14 @@

;; Lock up some uSTX for stacking! Note that the given amount here is in micro-STX (uSTX).
;; The STX will be locked for the given number of reward cycles (lock-period).
;; This is the self-service interface. tx-sender will be the Stacker.
;; This is the self-service interface. tx-sender will be the Stacker. Stacker is responsible for providing a valid signing key.
;;
;; * The given stacker cannot currently be stacking.
;; * You will need the minimum uSTX threshold. This will be determined by (get-stacking-minimum)
;; at the time this method is called.
;; * You may need to increase the amount of uSTX locked up later, since the minimum uSTX threshold
;; may increase between reward cycles.
;; * You need to provide a signing-key used to allocate signature-weight to a valid signer
;; * The Stacker will receive rewards in the reward cycle following `start-burn-ht`.
;; Importantly, `start-burn-ht` may not be further into the future than the next reward cycle,
;; and in most cases should be set to the current burn block height.
Expand All @@ -591,7 +593,8 @@
(define-public (stack-stx (amount-ustx uint)
(pox-addr (tuple (version (buff 1)) (hashbytes (buff 32))))
(start-burn-ht uint)
(lock-period uint))
(lock-period uint)
(signing-key (buff 33)))
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment about signing-key to the method doc?

Copy link
Collaborator Author

@setzeus setzeus Dec 14, 2023

Choose a reason for hiding this comment

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

Updated a sentence & added the following to comment doc above function:

;; ...Stacker is responsible for providing a valid signing key.
...
;; You need to provide a signing-key used to allocate signature-weight to a valid signer

;; this stacker's first reward cycle is the _next_ reward cycle
(let ((first-reward-cycle (+ u1 (current-pox-reward-cycle)))
(specified-reward-cycle (+ u1 (burn-height-to-reward-cycle start-burn-ht))))
Expand Down Expand Up @@ -628,7 +631,8 @@
reward-set-indexes: reward-set-indexes,
first-reward-cycle: first-reward-cycle,
lock-period: lock-period,
delegated-to: none })
delegated-to: none,
signing-key: signing-key })

;; return the lock-up information, so the node can actually carry out the lock.
(ok { stacker: tx-sender, lock-amount: amount-ustx, unlock-burn-height: (reward-cycle-to-burn-height (+ first-reward-cycle lock-period)) }))))
Expand Down Expand Up @@ -824,12 +828,14 @@
(ok true))))

;; As a delegate, stack the given principal's STX using partial-stacked-by-cycle
;; As a delegate, you need to provide a signing-key used to allocate signature-weight to a valid signer
;; Once the delegate has stacked > minimum, the delegate should call stack-aggregation-commit
(define-public (delegate-stack-stx (stacker principal)
(amount-ustx uint)
(pox-addr { version: (buff 1), hashbytes: (buff 32) })
(start-burn-ht uint)
(lock-period uint))
(lock-period uint)
Copy link
Member

Choose a reason for hiding this comment

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

Same here -- can you add a comment to the method doc about the signing key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated a sentence & added the following to comment doc above function:

;; * You need to provide a signing-key used to allocate signature-weight to a valid signer

(signing-key (buff 33)))
Copy link
Member

@moodmosaic moodmosaic Dec 21, 2023

Choose a reason for hiding this comment

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

@jcnelson, we'd like to get this method (delegate-stack-stx) under test, with @setzeus. In this diff, there's the signing-key key that's added.

Looking at PoX-3 tests, some candidates to copy over to PoX-4 are

All of these tests seem to call into delegate-stack-stx. The question is:

  1. Are all those tests relevant in the context of PoX-4
  2. Which one would be the best to start with, so it can be added into this PR.

;; this stacker's first reward cycle is the _next_ reward cycle
(let ((first-reward-cycle (+ u1 (current-pox-reward-cycle)))
(specified-reward-cycle (+ u1 (burn-height-to-reward-cycle start-burn-ht)))
Expand Down Expand Up @@ -885,7 +891,8 @@
first-reward-cycle: first-reward-cycle,
reward-set-indexes: (list),
lock-period: lock-period,
delegated-to: (some tx-sender) })
delegated-to: (some tx-sender),
signing-key: signing-key })

;; return the lock-up information, so the node can actually carry out the lock.
(ok { stacker: stacker,
Expand Down Expand Up @@ -1022,12 +1029,14 @@
;; This method extends the `tx-sender`'s current lockup for an additional `extend-count`
;; and associates `pox-addr` with the rewards
(define-public (stack-extend (extend-count uint)
(pox-addr { version: (buff 1), hashbytes: (buff 32) }))
(pox-addr { version: (buff 1), hashbytes: (buff 32) })
(updated-signing-key (optional (buff 33))))
(let ((stacker-info (stx-account tx-sender))
;; to extend, there must already be an etry in the stacking-state
;; to extend, there must already be an entry in the stacking-state
(stacker-state (unwrap! (get-stacker-info tx-sender) (err ERR_STACK_EXTEND_NOT_LOCKED)))
(amount-ustx (get locked stacker-info))
(unlock-height (get unlock-height stacker-info))
(next-signing-key (default-to (get signing-key stacker-state) updated-signing-key))
(cur-cycle (current-pox-reward-cycle))
;; first-extend-cycle will be the cycle in which tx-sender *would have* unlocked
(first-extend-cycle (burn-height-to-reward-cycle unlock-height))
Expand Down Expand Up @@ -1093,7 +1102,8 @@
reward-set-indexes: reward-set-indexes,
first-reward-cycle: first-reward-cycle,
lock-period: lock-period,
delegated-to: none })
delegated-to: none,
signing-key: next-signing-key })

;; return lock-up information
(ok { stacker: tx-sender, unlock-burn-height: new-unlock-ht })))))
Expand Down Expand Up @@ -1194,12 +1204,14 @@
(define-public (delegate-stack-extend
(stacker principal)
(pox-addr { version: (buff 1), hashbytes: (buff 32) })
(extend-count uint))
(extend-count uint)
(updated-signing-key (optional (buff 33))))
(let ((stacker-info (stx-account stacker))
;; to extend, there must already be an entry in the stacking-state
(stacker-state (unwrap! (get-stacker-info stacker) (err ERR_STACK_EXTEND_NOT_LOCKED)))
(amount-ustx (get locked stacker-info))
(unlock-height (get unlock-height stacker-info))
(next-signing-key (default-to (get signing-key stacker-state) updated-signing-key))
;; first-extend-cycle will be the cycle in which tx-sender *would have* unlocked
(first-extend-cycle (burn-height-to-reward-cycle unlock-height))
(cur-cycle (current-pox-reward-cycle))
Expand Down Expand Up @@ -1275,7 +1287,8 @@
reward-set-indexes: (list),
first-reward-cycle: first-reward-cycle,
lock-period: lock-period,
delegated-to: (some tx-sender) })
delegated-to: (some tx-sender),
signing-key: next-signing-key })

;; return the lock-up information, so the node can actually carry out the lock.
(ok { stacker: stacker,
Expand Down
25 changes: 22 additions & 3 deletions stackslib/src/chainstate/stacks/boot/pox_4_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ use crate::chainstate::stacks::boot::pox_2_tests::{
get_stacking_state_pox_2, get_stx_account_at, PoxPrintFields, StackingStateCheckData,
};
use crate::chainstate::stacks::boot::{
BOOT_CODE_COST_VOTING_TESTNET as BOOT_CODE_COST_VOTING, BOOT_CODE_POX_TESTNET, POX_2_NAME,
POX_3_NAME,
Compressed, Point, BOOT_CODE_COST_VOTING_TESTNET as BOOT_CODE_COST_VOTING,
BOOT_CODE_POX_TESTNET, POX_2_NAME, POX_3_NAME,
};
use crate::chainstate::stacks::db::{
MinerPaymentSchedule, StacksChainState, StacksHeaderInfo, MINER_REWARD_MATURITY,
Expand Down Expand Up @@ -213,9 +213,19 @@ fn pox_extend_transition() {
Some((EXPECTED_FIRST_V2_CYCLE, EXPECTED_FIRST_V2_CYCLE + 10));

let alice = keys.pop().unwrap();
let bob = keys.pop().unwrap();
let alice_pubkey = StacksPublicKey::from_private(&alice);
let alice_compressed = Compressed::try_from(alice_pubkey.to_bytes_compressed().as_slice())
.expect("Failed to convert public key to compressed struct");
let alice_point =
Point::try_from(&alice_compressed).expect("Failed to convert Alice's public key to point");
let alice_address = key_to_stacks_addr(&alice);
let alice_principal = PrincipalData::from(alice_address.clone());
let bob = keys.pop().unwrap();
let bob_pubkey = StacksPublicKey::from_private(&bob);
let bob_compressed = Compressed::try_from(bob_pubkey.to_bytes_compressed().as_slice())
.expect("Failed to convert public key to compressed struct");
let bob_point =
Point::try_from(&bob_compressed).expect("Failed to convert Bob's public key to point");
let bob_address = key_to_stacks_addr(&bob);
let bob_principal = PrincipalData::from(bob_address.clone());

Expand Down Expand Up @@ -486,6 +496,7 @@ fn pox_extend_transition() {
),
4,
tip.block_height,
&alice_point,
);
let alice_pox_4_lock_nonce = 2;
let alice_first_pox_4_unlock_height =
Expand Down Expand Up @@ -540,6 +551,7 @@ fn pox_extend_transition() {
),
3,
tip.block_height,
&bob_point,
);

// Alice can stack-extend in PoX v2
Expand All @@ -551,6 +563,7 @@ fn pox_extend_transition() {
key_to_stacks_addr(&alice).bytes,
),
6,
Some(&alice_point),
);

let alice_pox_4_extend_nonce = 3;
Expand Down Expand Up @@ -797,6 +810,11 @@ fn pox_lock_unlock() {
AddressHashMode::SerializeP2WSH,
])
.map(|(key, hash_mode)| {
let key_public = StacksPublicKey::from_private(key);
let key_compressed = Compressed::try_from(key_public.to_bytes_compressed().as_slice())
.expect("Failed to convert public key to compressed struct");
let key_point = Point::try_from(&key_compressed)
.expect("Failed to convert Alice's public key to point");
let pox_addr = PoxAddress::from_legacy(hash_mode, key_to_stacks_addr(key).bytes);
txs.push(make_pox_4_lockup(
key,
Expand All @@ -805,6 +823,7 @@ fn pox_lock_unlock() {
pox_addr.clone(),
lock_period,
tip_height,
&key_point,
));
pox_addr
})
Expand Down