[sui 7/x] - contract upgrades, version control (#762)

* state getters and setters, change Move.toml dependency to sui/integration_v2

* finish state.move

* add new line to pyth

* use deployer cap pattern for state module

* sui pyth

* update price feeds, dynamic object fields, Sui object PriceInfoObject

* register price info object with pyth state after creation

* sui governance

* some newlines

* error codes

* update and comment

* unit tests for pyth.move, add UpgradeCap to Pyth State (will be used for contract upgrades)

* updates

* test_get_update_fee test passes

* fix test_get_update_fee and test_update_price_feeds_corrupt_vaa

* test_update_price_feeds_invalid_data_source

* test_create_and_update_price_feeds

* test_create_and_update_price_feeds_success and test_create_and_update_price_feeds_price_info_object_not_found_failure

* test_update_cache

* update

* test_update_cache_old_update

* update_price_feeds_if_fresh

* comment

* contract upgrades start

* contract upgradeability

* update clock stuff

* edits

* use clone of sui/integration_v2 for stability

* make contract_upgrade::execute a public(friend) fun, remove clock arg

* E_INCORRECT_IDENTIFIER_LENGTH

* comment and edit

* add a single comment

Author: optke3

target_chains/sui/contracts/Move.toml

@@ -5,12 +5,12 @@ version = "0.0.1"
 [dependencies.Sui]
 git = "https://github.com/MystenLabs/sui.git"
 subdir = "crates/sui-framework/packages/sui-framework"
-rev = "ddfc3fa0768a38286787319603a5458a9ff91cc1"
+rev = "a63f425b9999c7fdfe483598720a9effc0acdc9e"
 
 [dependencies.Wormhole]
 git = "https://github.com/wormhole-foundation/wormhole.git"
 subdir = "sui/wormhole"
-rev = "sui/integration_v2"
+rev = "sui/integration_v2_stable"
 
 [addresses]
 pyth = "0x250"

target_chains/sui/contracts/sources/batch_price_attestation.move

@@ -165,14 +165,11 @@ module pyth::batch_price_attestation {
     fun test_deserialize_batch_price_attestation_invalid_magic() {
         use sui::test_scenario::{Self, take_shared, return_shared, ctx};
         let test = test_scenario::begin(@0x1234);
-        clock::create_for_testing(ctx(&mut test));
-        test_scenario::next_tx(&mut test, @0x1234);
-        let test_clock = take_shared<Clock>(&test);
-
+        let test_clock = clock::create_for_testing(ctx(&mut test));
         // A batch price attestation with a magic number of 0x50325749
         let bytes = x"5032574900030000000102000400951436e0be37536be96f0896366089506a59763d036728332d3e3038047851aea7c6c75c89f14810ec1c54c03ab8f1864a4c4032791f05747f560faec380a695d1000000000000049a0000000000000008fffffffb00000000000005dc0000000000000003000000000100000001000000006329c0eb000000006329c0e9000000006329c0e400000000000006150000000000000007215258d81468614f6b7e194c5d145609394f67b041e93e6695dcc616faadd0603b9551a68d01d954d6387aff4df1529027ffb2fee413082e509feb29cc4904fe000000000000041a0000000000000003fffffffb00000000000005cb0000000000000003010000000100000001000000006329c0eb000000006329c0e9000000006329c0e4000000000000048600000000000000078ac9cf3ab299af710d735163726fdae0db8465280502eb9f801f74b3c1bd190333832fad6e36eb05a8972fe5f219b27b5b2bb2230a79ce79beb4c5c5e7ecc76d00000000000003f20000000000000002fffffffb00000000000005e70000000000000003010000000100000001000000006329c0eb000000006329c0e9000000006329c0e40000000000000685000000000000000861db714e9ff987b6fedf00d01f9fea6db7c30632d6fc83b7bc9459d7192bc44a21a28b4c6619968bd8c20e95b0aaed7df2187fd310275347e0376a2cd7427db800000000000006cb0000000000000001fffffffb00000000000005e40000000000000003010000000100000001000000006329c0eb000000006329c0e9000000006329c0e400000000000007970000000000000001";
         let _ = destroy(deserialize(bytes, &test_clock));
-        return_shared(test_clock);
+        clock::destroy_for_testing(test_clock);
         test_scenario::end(test);
     }
 
@@ -181,9 +178,8 @@ module pyth::batch_price_attestation {
         use sui::test_scenario::{Self, take_shared, return_shared, ctx};
         // Set the arrival time
         let test = test_scenario::begin(@0x1234);
-        clock::create_for_testing(ctx(&mut test));
+        let test_clock = clock::create_for_testing(ctx(&mut test));
         test_scenario::next_tx(&mut test, @0x1234);
-        let test_clock = take_shared<Clock>(&test);
         let arrival_time_in_seconds = clock::timestamp_ms(&test_clock) / 1000;
 
         // let arrival_time = tx_context::epoch(ctx(&mut test));
@@ -244,7 +240,7 @@ module pyth::batch_price_attestation {
         assert!(&expected == &deserialized, 1);
         destroy(expected);
         destroy(deserialized);
-        return_shared(test_clock);
+        clock::destroy_for_testing(test_clock);
         test_scenario::end(test);
     }
 }

target_chains/sui/contracts/sources/governance/contract_upgrade.move

@@ -1,12 +1,101 @@
+// SPDX-License-Identifier: Apache 2
+
+/// Note: This module is based on the upgrade_contract module
+/// from the Sui Wormhole package:
+/// https://github.com/wormhole-foundation/wormhole/blob/sui/integration_v2/sui/wormhole/sources/governance/upgrade_contract.move
+
+/// This module implements handling a governance VAA to enact upgrading the
+/// Pyth contract to a new build. The procedure to upgrade this contract
+/// requires a Programmable Transaction, which includes the following procedure:
+/// 1.  Load new build.
+/// 2.  Authorize upgrade.
+/// 3.  Upgrade.
+/// 4.  Commit upgrade.
 module pyth::contract_upgrade {
-    use pyth::state::{State};
+    use sui::event::{Self};
+    use sui::object::{Self, ID};
+    use sui::package::{UpgradeReceipt, UpgradeTicket};
+
+    use pyth::state::{Self, State};
 
-    use wormhole::state::{State as WormState};
+    use wormhole::bytes32::{Self, Bytes32};
+    use wormhole::cursor::{Self};
 
     friend pyth::governance;
 
-    /// Payload should be the bytes digest of the new contract.
-    public(friend) fun execute(_worm_state: &WormState, _pyth_state: &State, _payload: vector<u8>){
-        // TODO
+    /// Digest is all zeros.
+    const E_DIGEST_ZERO_BYTES: u64 = 0;
+    /// Specific governance payload ID (action) to complete upgrading the
+    /// contract.
+    const ACTION_UPGRADE_CONTRACT: u8 = 1;
+
+    // Event reflecting package upgrade.
+    struct ContractUpgraded has drop, copy {
+        old_contract: ID,
+        new_contract: ID
+    }
+
+    struct UpgradeContract {
+        digest: Bytes32
+    }
+
+    /// Redeem governance VAA to issue an `UpgradeTicket` for the upgrade given
+    /// a contract upgrade VAA. This governance message is only relevant for Sui
+    /// because a contract upgrade is only relevant to one particular network
+    /// (in this case Sui), whose build digest is encoded in this message.
+    ///
+    /// NOTE: This method is guarded by a minimum build version check. This
+    /// method could break backward compatibility on an upgrade.
+    public(friend) fun execute(
+        pyth_state: &mut State,
+        payload: vector<u8>,
+    ): UpgradeTicket {
+        // Proceed with processing new implementation version.
+        handle_upgrade_contract(pyth_state, payload)
+    }
+
+    fun handle_upgrade_contract(
+        pyth_state: &mut State,
+        payload: vector<u8>
+    ): UpgradeTicket {
+
+        let UpgradeContract { digest } = deserialize(payload);
+
+        state::authorize_upgrade(pyth_state, digest)
+    }
+
+    /// Finalize the upgrade that ran to produce the given `receipt`. This
+    /// method invokes `state::commit_upgrade` which interacts with
+    /// `sui::package`.
+    public fun commit_upgrade(
+        self: &mut State,
+        receipt: UpgradeReceipt,
+    ) {
+        let latest_package_id = state::commit_upgrade(self, receipt);
+
+        // Emit an event reflecting package ID change.
+        event::emit(
+            ContractUpgraded {
+                old_contract: object::id_from_address(@pyth),
+                new_contract: latest_package_id
+            }
+        );
+    }
+
+    fun deserialize(payload: vector<u8>): UpgradeContract {
+        let cur = cursor::new(payload);
+
+        // This amount cannot be greater than max u64.
+        let digest = bytes32::take_bytes(&mut cur);
+        assert!(bytes32::is_nonzero(&digest), E_DIGEST_ZERO_BYTES);
+
+        cursor::destroy_empty(cur);
+
+        UpgradeContract { digest }
+    }
+
+    #[test_only]
+    public fun action(): u8 {
+        ACTION_UPGRADE_CONTRACT
     }
 }

target_chains/sui/contracts/sources/governance/governance.move

@@ -1,5 +1,7 @@
 module pyth::governance {
     use sui::clock::{Clock};
+    use sui::package::{UpgradeTicket};
+    use sui::tx_context::{TxContext};
 
     use pyth::data_source::{Self};
     use pyth::governance_instruction;
@@ -8,6 +10,7 @@ module pyth::governance {
     use pyth::set_governance_data_source;
     use pyth::set_data_sources;
     use pyth::set_stale_price_threshold;
+    use pyth::transfer_fee;
     use pyth::state::{State};
     use pyth::set_update_fee;
     use pyth::state;
@@ -18,23 +21,47 @@ module pyth::governance {
     const E_GOVERNANCE_CONTRACT_UPGRADE_CHAIN_ID_ZERO: u64 = 0;
     const E_INVALID_GOVERNANCE_ACTION: u64 = 1;
     const E_INVALID_GOVERNANCE_DATA_SOURCE: u64 = 2;
-    const E_INVALID_GOVERNANCE_SEQUENCE_NUMBER: u64 = 3;
+    const E_MUST_USE_EXECUTE_CONTRACT_UPGRADE_GOVERNANCE_INSTRUCTION_CALLSITE: u64 = 3;
+    const E_GOVERNANCE_ACTION_MUST_BE_CONTRACT_UPGRADE: u64 = 4;
 
-    public entry fun execute_governance_instruction(
+    /// Rather than having execute_governance_instruction handle the contract
+    /// upgrade governance instruction, we have a separate function that processes
+    /// contract upgrade instructions, because doing contract upgrades is a
+    /// multi-step process, and the first step of doing a contract upgrade
+    /// yields a return value, namely the upgrade ticket, which is non-droppable.
+    public fun execute_contract_upgrade_governance_instruction(
         pyth_state : &mut State,
         worm_state: &WormState,
         vaa_bytes: vector<u8>,
         clock: &Clock
+    ): UpgradeTicket {
+        let parsed_vaa = parse_and_verify_and_replay_protect_governance_vaa(pyth_state, worm_state, vaa_bytes, clock);
+        let instruction = governance_instruction::from_byte_vec(vaa::take_payload(parsed_vaa));
+        let action = governance_instruction::get_action(&instruction);
+        assert!(action == governance_action::new_contract_upgrade(),
+            E_GOVERNANCE_ACTION_MUST_BE_CONTRACT_UPGRADE);
+        assert!(governance_instruction::get_target_chain_id(&instruction) != 0,
+            E_GOVERNANCE_CONTRACT_UPGRADE_CHAIN_ID_ZERO);
+        contract_upgrade::execute(pyth_state, governance_instruction::destroy(instruction))
+    }
+
+    /// Execute a governance instruction.
+    public entry fun execute_governance_instruction(
+        pyth_state : &mut State,
+        worm_state: &WormState,
+        vaa_bytes: vector<u8>,
+        clock: &Clock,
+        ctx: &mut TxContext
     ) {
-        let parsed_vaa = parse_and_verify_governance_vaa(pyth_state, worm_state, vaa_bytes, clock);
+        let parsed_vaa = parse_and_verify_and_replay_protect_governance_vaa(pyth_state, worm_state, vaa_bytes, clock);
         let instruction = governance_instruction::from_byte_vec(vaa::take_payload(parsed_vaa));
 
-        // Dispatch the instruction to the appropiate handler
+        // Get the governance action.
         let action = governance_instruction::get_action(&instruction);
+
+        // Dispatch the instruction to the appropiate handler.
         if (action == governance_action::new_contract_upgrade()) {
-            assert!(governance_instruction::get_target_chain_id(&instruction) != 0,
-                E_GOVERNANCE_CONTRACT_UPGRADE_CHAIN_ID_ZERO);
-            contract_upgrade::execute(worm_state, pyth_state, governance_instruction::destroy(instruction));
+            abort(E_MUST_USE_EXECUTE_CONTRACT_UPGRADE_GOVERNANCE_INSTRUCTION_CALLSITE)
         } else if (action == governance_action::new_set_governance_data_source()) {
             set_governance_data_source::execute(pyth_state, governance_instruction::destroy(instruction));
         } else if (action == governance_action::new_set_data_sources()) {
@@ -43,13 +70,15 @@ module pyth::governance {
             set_update_fee::execute(pyth_state, governance_instruction::destroy(instruction));
         } else if (action == governance_action::new_set_stale_price_threshold()) {
             set_stale_price_threshold::execute(pyth_state, governance_instruction::destroy(instruction));
+        } else if (action == governance_action::new_transfer_fee()) {
+            transfer_fee::execute(pyth_state, governance_instruction::destroy(instruction), ctx);
         } else {
             governance_instruction::destroy(instruction);
             assert!(false, E_INVALID_GOVERNANCE_ACTION);
         }
     }
 
-    fun parse_and_verify_governance_vaa(
+    fun parse_and_verify_and_replay_protect_governance_vaa(
         pyth_state: &mut State,
         worm_state: &WormState,
         bytes: vector<u8>,
@@ -66,11 +95,8 @@ module pyth::governance {
                     vaa::emitter_address(&parsed_vaa))),
             E_INVALID_GOVERNANCE_DATA_SOURCE);
 
-        // Check that the sequence number is greater than the last executed governance VAA
-        let sequence = vaa::sequence(&parsed_vaa);
-        assert!(sequence > state::get_last_executed_governance_sequence(pyth_state), E_INVALID_GOVERNANCE_SEQUENCE_NUMBER);
-        state::set_last_executed_governance_sequence(pyth_state, sequence);
-
+        // Prevent replay attacks by consuming the VAA digest (adding it to a set)
+        state::consume_vaa(pyth_state, vaa::digest(&parsed_vaa));
         parsed_vaa
     }
 }

target_chains/sui/contracts/sources/governance/governance_action.move

@@ -6,6 +6,7 @@ module pyth::governance_action {
     const SET_DATA_SOURCES: u8 = 2;
     const SET_UPDATE_FEE: u8 = 3;
     const SET_STALE_PRICE_THRESHOLD: u8 = 4;
+    const TRANSFER_FEE: u8 = 5;
 
     const E_INVALID_GOVERNANCE_ACTION: u64 = 5;
 
@@ -37,4 +38,8 @@ module pyth::governance_action {
     public fun new_set_stale_price_threshold(): GovernanceAction {
         GovernanceAction { value: SET_STALE_PRICE_THRESHOLD }
     }
+
+    public fun new_transfer_fee(): GovernanceAction {
+        GovernanceAction { value: TRANSFER_FEE }
+    }
 }

target_chains/sui/contracts/sources/governance/set_data_sources.move

@@ -8,19 +8,22 @@ module pyth::set_data_sources {
     use pyth::deserialize;
     use pyth::data_source::{Self, DataSource};
     use pyth::state::{Self, State};
+    use pyth::version_control::{SetDataSources};
 
     friend pyth::governance;
 
-    struct SetDataSources {
+    struct DataSources {
         sources: vector<DataSource>,
     }
 
     public(friend) fun execute(state: &mut State, payload: vector<u8>) {
-        let SetDataSources { sources } = from_byte_vec(payload);
+        state::check_minimum_requirement<SetDataSources>(state);
+
+        let DataSources { sources } = from_byte_vec(payload);
         state::set_data_sources(state, sources);
     }
 
-    fun from_byte_vec(bytes: vector<u8>): SetDataSources {
+    fun from_byte_vec(bytes: vector<u8>): DataSources {
         let cursor = cursor::new(bytes);
         let data_sources_count = deserialize::deserialize_u8(&mut cursor);
 
@@ -37,7 +40,7 @@ module pyth::set_data_sources {
 
         cursor::destroy_empty(cursor);
 
-        SetDataSources {
+        DataSources {
             sources
         }
     }

target_chains/sui/contracts/sources/governance/set_governance_data_source.move

@@ -2,33 +2,35 @@ module pyth::set_governance_data_source {
     use pyth::deserialize;
     use pyth::data_source;
     use pyth::state::{Self, State};
+    use pyth::version_control::SetGovernanceDataSource;
 
     use wormhole::cursor;
     use wormhole::external_address::{Self, ExternalAddress};
     use wormhole::bytes32::{Self};
-    //use wormhole::state::{Self}
 
     friend pyth::governance;
 
-    struct SetGovernanceDataSource {
+    struct GovernanceDataSource {
         emitter_chain_id: u64,
         emitter_address: ExternalAddress,
         initial_sequence: u64,
     }
 
     public(friend) fun execute(pyth_state: &mut State, payload: vector<u8>) {
-        let SetGovernanceDataSource { emitter_chain_id, emitter_address, initial_sequence } = from_byte_vec(payload);
+        state::check_minimum_requirement<SetGovernanceDataSource>(pyth_state);
+
+        // TODO - What is GovernanceDataSource initial_sequence used for?
+        let GovernanceDataSource { emitter_chain_id, emitter_address, initial_sequence: _initial_sequence } = from_byte_vec(payload);
         state::set_governance_data_source(pyth_state, data_source::new(emitter_chain_id, emitter_address));
-        state::set_last_executed_governance_sequence(pyth_state, initial_sequence);
     }
 
-    fun from_byte_vec(bytes: vector<u8>): SetGovernanceDataSource {
+    fun from_byte_vec(bytes: vector<u8>): GovernanceDataSource {
         let cursor = cursor::new(bytes);
         let emitter_chain_id = deserialize::deserialize_u16(&mut cursor);
         let emitter_address = external_address::new(bytes32::from_bytes(deserialize::deserialize_vector(&mut cursor, 32)));
         let initial_sequence = deserialize::deserialize_u64(&mut cursor);
         cursor::destroy_empty(cursor);
-        SetGovernanceDataSource {
+        GovernanceDataSource {
             emitter_chain_id: (emitter_chain_id as u64),
             emitter_address,
             initial_sequence

target_chains/sui/contracts/sources/governance/set_stale_price_threshold.move

@@ -2,23 +2,26 @@ module pyth::set_stale_price_threshold {
     use wormhole::cursor;
     use pyth::deserialize;
     use pyth::state::{Self, State};
+    use pyth::version_control::SetStalePriceThreshold;
 
     friend pyth::governance;
 
-    struct SetStalePriceThreshold {
+    struct StalePriceThreshold {
         threshold: u64,
     }
 
     public(friend) fun execute(state: &mut State, payload: vector<u8>) {
-        let SetStalePriceThreshold { threshold } = from_byte_vec(payload);
+        state::check_minimum_requirement<SetStalePriceThreshold>(state);
+
+        let StalePriceThreshold { threshold } = from_byte_vec(payload);
         state::set_stale_price_threshold_secs(state, threshold);
     }
 
-    fun from_byte_vec(bytes: vector<u8>): SetStalePriceThreshold {
+    fun from_byte_vec(bytes: vector<u8>): StalePriceThreshold {
         let cursor = cursor::new(bytes);
         let threshold = deserialize::deserialize_u64(&mut cursor);
         cursor::destroy_empty(cursor);
-        SetStalePriceThreshold {
+        StalePriceThreshold {
             threshold
         }
     }