Implement PoX-4 Locking via Special Contract-Call Handler

[jbencin]

Description

Copy/paste pox_3.rs to pox_4.rs and rename things as needed. Also, fix clippy::needless_return lint

Applicable issues

• fixes [Nakamoto] Add pox_locking::pox_4 lockup methods #4090
• fixes [Nakamoto] Update handle_contract_call_special_cases() in stackslib/src/clarity_vm/special.rs to handle PoX-4 #4091

Additional info (benefits, drawbacks, caveats)

Checklist

• Port the following pox3 tests to pox4
  • delegate_extend_pox_3
  • delegate_stack_increase
  • get_pox_addrs -> pox_lock_unlock
  • pox_3_delegate_stx_addr_validation
  • pox_3_getters
  • pox_auto_unlock_ab
  • pox_auto_unlock_ba
  • pox_extend_transition
  • simple_pox_lockup_transition_pox_2
  • stack_aggregation_increase
  • stack_increase
  • stack_with_segwit
• Test that pox3 methods fail (pox_3_fails)
• Test that STX locked in pox3 unlocks

[codecov]

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (9fc7617) 17.17% compared to head (b747597) 38.23%.
Report is 21 commits behind head on next.

Files	Patch %	Lines
pox-locking/src/pox_3.rs	70.83%	7 Missing ⚠️
...kslib/src/chainstate/nakamoto/coordinator/tests.rs	53.33%	7 Missing ⚠️
clarity/src/vm/database/structures.rs	50.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             next    #4106       +/-   ##
===========================================
+ Coverage   17.17%   38.23%   +21.06%     
===========================================
  Files         418      418               
  Lines      295319   295319               
===========================================
+ Hits        50718   112921    +62203     
+ Misses     244601   182398    -62203     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hugocaillard]

Following this issue on Clarinet: hirosystems/clarinet#1267, we brainstormed with @lgalabru for a clean way to handle the pox-4.clar contract on Clarinet (regarding the concatenation with pox-<netowrk>.clar).

While we all agree that it would be nice to have a standard way to handle such use cases at the Clarity or Clarinet level, it may take a bit of time to get there.

In the meantime, regarding the pox-4 contract specifically, what about leveraging is-in-mainnet that was introduced in Clarity 2? With the constants defined based on that, it would completely remove the need for contract concatenation.

(define-constant PREPARE_CYCLE_LENGTH (if is-in-mainnet u100 u50))

[jbencin]

@moodmosaic I managed to fix one test get_pox_addrs() by doing the following:

• Disable epoch 3.0 and nakamoto blocks. We can test pox4 in Epoch 2.5
• Experiment with stacking start height. We submit the stacking txs at first_v4_cycle - 3
• Experiment with when we start checking things, at first_v4_cycle + 5

I still don't understand why these block heights work, but the tests clearly show STX are being locked for a couple cycles, then unlocked, so it looks okay to me

Also, going to give you write access to this branch so you can push (since it's in my repo)

[jcnelson]

Per our meeting earlier, I think the only things that this file needs to test are:

• PoX 4 instantiates
• STX locked in PoX 3 unlock automatically
• Once PoX 4 is active, only PoX 4 stacking methods work. PoX 3 ones fail.

There's no reason to test PoX 2 and PoX behavior, since both these contracts have been defunct for some time.

[moodmosaic]

@jcnelson, @kantai

• PoX 4 instantiates

does get_pox_addrs captures this? If it does, I would like to start the current pox_4_tests.rs (in this PR) from a clean state and start by just adding this test, and then keep adding to it.

[kantai]

LGTM -- I would just recommend using from_seed to initialize the necessary new StacksPrivateKeys in nakamoto::coordinator::tests

[jcnelson]

Can you check the tx receipts to verify that stacking actually failed because pox-3 was defunct?

[jbencin]

Trying to figure out how to do this, I think I understand, but want to check:

• Get StacksTransactionReceipts from latest block after executing transactions
• Filter the ones that contain a StacksTransactionEvent with a pox-3 call
• Check that StacksTransactionReceipt.result is an error value

Does this sound right?

[jcnelson]

Yes; if possible, also test that we specifically fail because contract was defunct (as opposed to some other error that might arise due to a regression in the test).

[kantai]

I'm not sure that the StacksTransactionReceipts will be very discerning for these checks -- runtime errors (like these should be) just return a receipt with a result of err_none, an empty assetmap, and an empty event list. But, you could usefully assert that a runtime error occurs by checking the receipt.

This is how pox_3_tests does this:

https://github.com/stacks-network/stacks-core/blob/develop/stackslib/src/chainstate/stacks/boot/pox_3_tests.rs#L512

[jbencin]

I'm actually seeing something like this in StacksTransactionReceipt.result:

(ok (tuple (lock-amount u10240000000000) (stacker ST3DYDR173ZWF79YJ40JEHDPVAR2815PDZ0PF8C67) (unlock-burn-height u125)))

So the contract call looks to be returning okay, but the other checks suggest that the STX are not actually locked

[kantai]

Ah, that's because pox-3 doesn't seem to actually be marked defunct in these changes. You need to do something like:

diff --git a/pox-locking/src/lib.rs b/pox-locking/src/lib.rs
index 05303a4d9..057667341 100644
--- a/pox-locking/src/lib.rs
+++ b/pox-locking/src/lib.rs
@@ -107,6 +107,19 @@ pub fn handle_contract_call_special_cases(
             result,
         );
     } else if *contract_id == boot_code_id(POX_3_NAME, global_context.mainnet) {
+        if !pox_3::is_read_only(function_name) && global_context.epoch_id >= StacksEpochId::Epoch25
+        {
+            warn!("PoX-3 function call attempted on an account after Epoch 2.5";
+                  "v3_unlock_ht" => global_context.database.get_v3_unlock_height(),
+                  "current_burn_ht" => global_context.database.get_current_burnchain_block_height(),
+                  "function_name" => function_name,
+                  "contract_id" => %contract_id
+            );
+            return Err(ClarityError::Runtime(
+                RuntimeErrorType::DefunctPoxContract,
+                None,
+            ));
+        }
         return pox_3::handle_contract_call(
             global_context,
             sender,
diff --git a/pox-locking/src/pox_3.rs b/pox-locking/src/pox_3.rs
index cccfbb264..e0110987e 100644
--- a/pox-locking/src/pox_3.rs
+++ b/pox-locking/src/pox_3.rs
@@ -32,6 +32,33 @@ use crate::pox_2::{parse_pox_extend_result, parse_pox_increase, parse_pox_stacki
 use crate::{LockingError, POX_3_NAME};
 
 /////////////////////// PoX-3 /////////////////////////////////
+/// is a PoX-3 function call read only?
+pub(crate) fn is_read_only(func_name: &str) -> bool {
+    "get-pox-rejection" == func_name
+        || "is-pox-active" == func_name
+        || "burn-height-to-reward-cycle" == func_name
+        || "reward-cycle-to-burn-height" == func_name
+        || "current-pox-reward-cycle" == func_name
+        || "get-stacker-info" == func_name
+        || "get-check-delegation" == func_name
+        || "get-reward-set-size" == func_name
+        || "next-cycle-rejection-votes" == func_name
+        || "get-total-ustx-stacked" == func_name
+        || "get-reward-set-pox-address" == func_name
+        || "get-stacking-minimum" == func_name
+        || "check-pox-addr-version" == func_name
+        || "check-pox-addr-hashbytes" == func_name
+        || "check-pox-lock-period" == func_name
+        || "can-stack-stx" == func_name
+        || "minimal-can-stack-stx" == func_name
+        || "get-pox-info" == func_name
+        || "get-delegation-info" == func_name
+        || "get-allowance-contract-callers" == func_name
+        || "get-num-reward-set-pox-addresses" == func_name
+        || "get-partial-stacked-by-cycle" == func_name
+        || "get-total-pox-rejection" == func_name
+}
+
 
 /// Lock up STX for PoX for a time.  Does NOT touch the account nonce.
 pub fn pox_lock_v3(

[jbencin]

Thanks @kantai, that fixed it! Will check in and merge this ASAP

[jcnelson]

LGTM; just please address my remaining concerns before merging. Thanks!

[kantai]

LGTM, just a couple tweaks to the test case.

[blockstack-devops]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.