Răsfoiți Sursa

feat(target_chains/starknet): governance data source transfer (#1589)

Pavel Strakhov 1 an în urmă
părinte
comite
627a606302

+ 76 - 6
target_chains/starknet/contracts/src/pyth.cairo

@@ -3,7 +3,7 @@ mod interface;
 mod price_update;
 mod governance;
 
-pub use pyth::{Event, PriceFeedUpdateEvent, WormholeAddressSet};
+pub use pyth::{Event, PriceFeedUpdateEvent, WormholeAddressSet, GovernanceDataSourceSet};
 pub use errors::{GetPriceUnsafeError, GovernanceActionError, UpdatePriceFeedsError};
 pub use interface::{IPyth, IPythDispatcher, IPythDispatcherTrait, DataSource, Price};
 
@@ -30,6 +30,7 @@ mod pyth {
     pub enum Event {
         PriceFeedUpdate: PriceFeedUpdateEvent,
         WormholeAddressSet: WormholeAddressSet,
+        GovernanceDataSourceSet: GovernanceDataSourceSet,
     }
 
     #[derive(Drop, PartialEq, starknet::Event)]
@@ -47,6 +48,13 @@ mod pyth {
         pub new_address: ContractAddress,
     }
 
+    #[derive(Drop, PartialEq, starknet::Event)]
+    pub struct GovernanceDataSourceSet {
+        pub old_data_source: DataSource,
+        pub new_data_source: DataSource,
+        pub last_executed_governance_sequence: u64,
+    }
+
     #[storage]
     struct Storage {
         wormhole_address: ContractAddress,
@@ -60,6 +68,9 @@ mod pyth {
         latest_price_info: LegacyMap<u256, PriceInfo>,
         governance_data_source: DataSource,
         last_executed_governance_sequence: u64,
+        // Governance data source index is used to prevent replay attacks,
+        // so a claimVaa cannot be used twice.
+        governance_data_source_index: u32,
     }
 
     /// Initializes the Pyth contract.
@@ -223,9 +234,16 @@ mod pyth {
                         old_address: wormhole.contract_address, new_address: payload.address,
                     };
                     self.emit(event);
-                }
+                },
+                GovernancePayload::RequestGovernanceDataSourceTransfer(_) => {
+                    // RequestGovernanceDataSourceTransfer can be only part of
+                    // AuthorizeGovernanceDataSourceTransfer message
+                    panic_with_felt252(GovernanceActionError::InvalidGovernanceMessage.into());
+                },
+                GovernancePayload::AuthorizeGovernanceDataSourceTransfer(payload) => {
+                    self.authorize_governance_transfer(payload.claim_vaa);
+                },
             }
-            self.last_executed_governance_sequence.write(vm.sequence);
         }
     }
 
@@ -278,7 +296,7 @@ mod pyth {
             self.single_update_fee.read() * num_updates.into()
         }
 
-        fn verify_governance_vm(self: @ContractState, vm: @VerifiedVM) {
+        fn verify_governance_vm(ref self: ContractState, vm: @VerifiedVM) {
             let governance_data_source = self.governance_data_source.read();
             if governance_data_source.emitter_chain_id != *vm.emitter_chain_id {
                 panic_with_felt252(GovernanceActionError::InvalidGovernanceDataSource.into());
@@ -289,14 +307,28 @@ mod pyth {
             if *vm.sequence <= self.last_executed_governance_sequence.read() {
                 panic_with_felt252(GovernanceActionError::OldGovernanceMessage.into());
             }
+            // Note: in case of AuthorizeGovernanceDataSourceTransfer,
+            // last_executed_governance_sequence is later overwritten with the value from claim_vaa
+            self.last_executed_governance_sequence.write(*vm.sequence);
         }
 
         fn check_new_wormhole(
-            self: @ContractState, wormhole_address: ContractAddress, vm: ByteArray
+            ref self: ContractState, wormhole_address: ContractAddress, vm: ByteArray
         ) {
             let wormhole = IWormholeDispatcher { contract_address: wormhole_address };
             let vm = wormhole.parse_and_verify_vm(vm);
-            self.verify_governance_vm(@vm);
+
+            let governance_data_source = self.governance_data_source.read();
+            if governance_data_source.emitter_chain_id != vm.emitter_chain_id {
+                panic_with_felt252(GovernanceActionError::InvalidGovernanceDataSource.into());
+            }
+            if governance_data_source.emitter_address != vm.emitter_address {
+                panic_with_felt252(GovernanceActionError::InvalidGovernanceDataSource.into());
+            }
+
+            if vm.sequence != self.last_executed_governance_sequence.read() {
+                panic_with_felt252(GovernanceActionError::InvalidWormholeAddressToSet.into());
+            }
             // Purposefully, we don't check whether the chainId is the same as the current chainId because
             // we might want to change the chain id of the wormhole contract.
             let data = governance::parse_instruction(vm.payload);
@@ -315,6 +347,44 @@ mod pyth {
                 }
             }
         }
+
+        fn authorize_governance_transfer(ref self: ContractState, claim_vaa: ByteArray) {
+            let wormhole = IWormholeDispatcher { contract_address: self.wormhole_address.read() };
+            let claim_vm = wormhole.parse_and_verify_vm(claim_vaa.clone());
+            // Note: no verify_governance_vm() because claim_vaa is signed by the new data source
+            let instruction = governance::parse_instruction(claim_vm.payload);
+            if instruction.target_chain_id != 0
+                && instruction.target_chain_id != wormhole.chain_id() {
+                panic_with_felt252(GovernanceActionError::InvalidGovernanceTarget.into());
+            }
+            let request_payload = match instruction.payload {
+                GovernancePayload::RequestGovernanceDataSourceTransfer(payload) => payload,
+                _ => { panic_with_felt252(GovernanceActionError::InvalidGovernanceMessage.into()) }
+            };
+            // Governance data source index is used to prevent replay attacks,
+            // so a claimVaa cannot be used twice.
+            let current_index = self.governance_data_source_index.read();
+            let new_index = request_payload.governance_data_source_index;
+            if current_index >= new_index {
+                panic_with_felt252(GovernanceActionError::OldGovernanceMessage.into());
+            }
+            self.governance_data_source_index.write(request_payload.governance_data_source_index);
+            let old_data_source = self.governance_data_source.read();
+            let new_data_source = DataSource {
+                emitter_chain_id: claim_vm.emitter_chain_id,
+                emitter_address: claim_vm.emitter_address,
+            };
+            self.governance_data_source.write(new_data_source);
+            // Setting the last executed governance to the claimVaa sequence to avoid
+            // using older sequences.
+            let last_executed_governance_sequence = claim_vm.sequence;
+            self.last_executed_governance_sequence.write(last_executed_governance_sequence);
+
+            let event = GovernanceDataSourceSet {
+                old_data_source, new_data_source, last_executed_governance_sequence,
+            };
+            self.emit(event);
+        }
     }
 
     fn apply_decimal_expo(value: u64, expo: u64) -> u256 {

+ 29 - 4
target_chains/starknet/contracts/src/pyth/governance.cairo

@@ -1,3 +1,4 @@
+use pyth::reader::ReaderTrait;
 use core::array::ArrayTrait;
 use pyth::reader::{Reader, ReaderImpl};
 use pyth::byte_array::ByteArray;
@@ -47,6 +48,8 @@ pub enum GovernancePayload {
     SetFee: SetFee,
     SetDataSources: SetDataSources,
     SetWormholeAddress: SetWormholeAddress,
+    RequestGovernanceDataSourceTransfer: RequestGovernanceDataSourceTransfer,
+    AuthorizeGovernanceDataSourceTransfer: AuthorizeGovernanceDataSourceTransfer,
 // TODO: others
 }
 
@@ -66,6 +69,21 @@ pub struct SetWormholeAddress {
     pub address: ContractAddress,
 }
 
+#[derive(Drop, Debug)]
+pub struct RequestGovernanceDataSourceTransfer {
+    // Index is used to prevent replay attacks
+    // So a claimVaa cannot be used twice.
+    pub governance_data_source_index: u32,
+}
+
+#[derive(Drop, Debug)]
+pub struct AuthorizeGovernanceDataSourceTransfer {
+    // Transfer governance control over this contract to another data source.
+    // The claim_vaa field is a VAA created by the new data source; using a VAA prevents mistakes
+    // in the handoff by ensuring that the new data source can send VAAs (i.e., is not an invalid address).
+    pub claim_vaa: ByteArray,
+}
+
 pub fn parse_instruction(payload: ByteArray) -> GovernanceInstruction {
     let mut reader = ReaderImpl::new(payload);
     let magic = reader.read_u32();
@@ -86,7 +104,17 @@ pub fn parse_instruction(payload: ByteArray) -> GovernanceInstruction {
     let payload = match action {
         GovernanceAction::UpgradeContract => { panic_with_felt252('unimplemented') },
         GovernanceAction::AuthorizeGovernanceDataSourceTransfer => {
-            panic_with_felt252('unimplemented')
+            let len = reader.len();
+            let claim_vaa = reader.read_byte_array(len);
+            GovernancePayload::AuthorizeGovernanceDataSourceTransfer(
+                AuthorizeGovernanceDataSourceTransfer { claim_vaa }
+            )
+        },
+        GovernanceAction::RequestGovernanceDataSourceTransfer => {
+            let governance_data_source_index = reader.read_u32();
+            GovernancePayload::RequestGovernanceDataSourceTransfer(
+                RequestGovernanceDataSourceTransfer { governance_data_source_index }
+            )
         },
         GovernanceAction::SetDataSources => {
             let num_sources = reader.read_u8();
@@ -106,9 +134,6 @@ pub fn parse_instruction(payload: ByteArray) -> GovernanceInstruction {
             GovernancePayload::SetFee(SetFee { value, expo })
         },
         GovernanceAction::SetValidPeriod => { panic_with_felt252('unimplemented') },
-        GovernanceAction::RequestGovernanceDataSourceTransfer => {
-            panic_with_felt252('unimplemented')
-        },
         GovernanceAction::SetWormholeAddress => {
             let address: felt252 = reader
                 .read_u256()

+ 1 - 1
target_chains/starknet/contracts/src/pyth/interface.cairo

@@ -11,7 +11,7 @@ pub trait IPyth<T> {
     fn execute_governance_instruction(ref self: T, data: ByteArray);
 }
 
-#[derive(Drop, Debug, Clone, Copy, Hash, Default, Serde, starknet::Store)]
+#[derive(Drop, Debug, Clone, Copy, PartialEq, Hash, Default, Serde, starknet::Store)]
 pub struct DataSource {
     pub emitter_chain_id: u16,
     pub emitter_address: u256,

+ 40 - 0
target_chains/starknet/contracts/tests/data.cairo

@@ -361,6 +361,46 @@ pub fn pyth_set_wormhole() -> ByteArray {
     ByteArrayImpl::new(array_try_into(bytes), 8)
 }
 
+// A Pyth governance instruction to request governance data source transfer signed by the test guardian #1.
+pub fn pyth_request_transfer() -> ByteArray {
+    let bytes = array![
+        1766847064779995673162446580588349917525470038054832932592992288867429640,
+        324153743158695484170440096219469152425935271630646363385677281984034639103,
+        9668785162708967120184333307045308321106903473741317705519937487712026624,
+        51983810243429054512432720,
+        101886477340929157123538945,
+    ];
+    ByteArrayImpl::new(array_try_into(bytes), 11)
+}
+
+// A Pyth governance instruction to authorize governance data source transfer signed by the test guardian #1.
+pub fn pyth_auth_transfer() -> ByteArray {
+    let bytes = array![
+        1766847064779996877169354131457289870145133774197236214231189828595607612,
+        135397929264511926704016137904543076941936590450380629211164830069940259166,
+        170704271745871732109596595409322690827425239986206012346500026141882974208,
+        49565958604199796163020368,
+        148907253454000398381888169517670981236167827299573051583944758214402801641,
+        366234460504415466174775710781157853773075726148887508605191363426437722203,
+        180989472713677925664001543732082345206417308361605690793789229681751359488,
+        721420288,
+        20782639266000304984163621011457,
+    ];
+    ByteArrayImpl::new(array_try_into(bytes), 18)
+}
+
+// A Pyth governance instruction to set fee with alternative emitter signed by the test guardian #1.
+pub fn pyth_set_fee_alt_emitter() -> ByteArray {
+    let bytes = array![
+        1766847064779994296698028907175740560793090837330420619723848454331748383,
+        249896185465136242775777270873107292725618014836270803491051657047070145541,
+        225947213663038958976782611621298954045962455954572977382945927572467220480,
+        51983810243429054512498256,
+        8072278384728444780182694421117884443886221966887092226,
+    ];
+    ByteArrayImpl::new(array_try_into(bytes), 23)
+}
+
 // An update pulled from Hermes and re-signed by the test guardian #1.
 pub fn test_price_update1() -> ByteArray {
     let bytes = array![

+ 91 - 22
target_chains/starknet/contracts/tests/pyth.cairo

@@ -4,7 +4,7 @@ use snforge_std::{
 };
 use pyth::pyth::{
     IPythDispatcher, IPythDispatcherTrait, DataSource, Event as PythEvent, PriceFeedUpdateEvent,
-    WormholeAddressSet,
+    WormholeAddressSet, GovernanceDataSourceSet,
 };
 use pyth::byte_array::{ByteArray, ByteArrayImpl};
 use pyth::util::{array_try_into, UnwrapWithFelt252};
@@ -14,32 +14,49 @@ use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDi
 use super::wormhole::corrupted_vm;
 use super::data;
 
-fn decode_event(event: @Event) -> PythEvent {
-    let key0 = *event.keys.at(0);
-    if key0 == event_name_hash('PriceFeedUpdate') {
-        assert!(event.keys.len() == 3);
-        assert!(event.data.len() == 3);
+#[generate_trait]
+impl DecodeEventHelpers of DecodeEventHelpersTrait {
+    fn pop<T, +TryInto<felt252, T>>(ref self: Array<felt252>) -> T {
+        self.pop_front().unwrap().try_into().unwrap()
+    }
+
+    fn pop_u256(ref self: Array<felt252>) -> u256 {
+        u256 { low: self.pop(), high: self.pop(), }
+    }
+
+    fn pop_data_source(ref self: Array<felt252>) -> DataSource {
+        DataSource { emitter_chain_id: self.pop(), emitter_address: self.pop_u256(), }
+    }
+}
+
+fn decode_event(mut event: Event) -> PythEvent {
+    let key0: felt252 = event.keys.pop();
+    let output = if key0 == event_name_hash('PriceFeedUpdate') {
         let event = PriceFeedUpdateEvent {
-            price_id: u256 {
-                low: (*event.keys.at(1)).try_into().unwrap(),
-                high: (*event.keys.at(2)).try_into().unwrap(),
-            },
-            publish_time: (*event.data.at(0)).try_into().unwrap(),
-            price: (*event.data.at(1)).try_into().unwrap(),
-            conf: (*event.data.at(2)).try_into().unwrap(),
+            price_id: event.keys.pop_u256(),
+            publish_time: event.data.pop(),
+            price: event.data.pop(),
+            conf: event.data.pop(),
         };
         PythEvent::PriceFeedUpdate(event)
     } else if key0 == event_name_hash('WormholeAddressSet') {
-        assert!(event.keys.len() == 1);
-        assert!(event.data.len() == 2);
         let event = WormholeAddressSet {
-            old_address: (*event.data.at(0)).try_into().unwrap(),
-            new_address: (*event.data.at(1)).try_into().unwrap(),
+            old_address: event.data.pop(), new_address: event.data.pop(),
         };
         PythEvent::WormholeAddressSet(event)
+    } else if key0 == event_name_hash('GovernanceDataSourceSet') {
+        let event = GovernanceDataSourceSet {
+            old_data_source: event.data.pop_data_source(),
+            new_data_source: event.data.pop_data_source(),
+            last_executed_governance_sequence: event.data.pop(),
+        };
+        PythEvent::GovernanceDataSourceSet(event)
     } else {
         panic!("unrecognized event")
-    }
+    };
+    assert!(event.keys.len() == 0);
+    assert!(event.data.len() == 0);
+    output
 }
 
 #[test]
@@ -62,8 +79,8 @@ fn update_price_feeds_works() {
 
     spy.fetch_events();
     assert!(spy.events.len() == 1);
-    let (from, event) = spy.events.at(0);
-    assert!(from == @pyth.contract_address);
+    let (from, event) = spy.events.pop_front().unwrap();
+    assert!(from == pyth.contract_address);
     let event = decode_event(event);
     let expected = PriceFeedUpdateEvent {
         price_id: 0xe62df6c8b4a85fe1a67db44dc12de5db330f7ac66b72dc658afedf0f4a415b43,
@@ -245,8 +262,8 @@ fn test_governance_set_wormhole_works() {
 
     spy.fetch_events();
     assert!(spy.events.len() == 1);
-    let (from, event) = spy.events.at(0);
-    assert!(from == @pyth.contract_address);
+    let (from, event) = spy.events.pop_front().unwrap();
+    assert!(from == pyth.contract_address);
     let event = decode_event(event);
     let expected = WormholeAddressSet {
         old_address: wormhole_address, new_address: wormhole2_address,
@@ -338,6 +355,58 @@ fn test_rejects_set_wormhole_with_incompatible_guardians() {
     pyth.execute_governance_instruction(data::pyth_set_wormhole());
 }
 
+#[test]
+fn test_governance_transfer_works() {
+    let owner = 'owner'.try_into().unwrap();
+    let user = 'user'.try_into().unwrap();
+    let wormhole = super::wormhole::deploy_with_test_guardian();
+    let fee_contract = deploy_fee_contract(user);
+    let pyth = deploy_default(owner, wormhole.contract_address, fee_contract.contract_address);
+
+    let mut spy = spy_events(SpyOn::One(pyth.contract_address));
+
+    pyth.execute_governance_instruction(data::pyth_auth_transfer());
+
+    spy.fetch_events();
+    assert!(spy.events.len() == 1);
+    let (from, event) = spy.events.pop_front().unwrap();
+    assert!(from == pyth.contract_address);
+    let event = decode_event(event);
+    let expected = GovernanceDataSourceSet {
+        old_data_source: DataSource { emitter_chain_id: 1, emitter_address: 41, },
+        new_data_source: DataSource { emitter_chain_id: 2, emitter_address: 43, },
+        last_executed_governance_sequence: 1,
+    };
+    assert!(event == PythEvent::GovernanceDataSourceSet(expected));
+
+    pyth.execute_governance_instruction(data::pyth_set_fee_alt_emitter());
+}
+
+#[test]
+#[should_panic(expected: ('invalid governance data source',))]
+fn test_set_fee_rejects_wrong_emitter() {
+    let owner = 'owner'.try_into().unwrap();
+    let user = 'user'.try_into().unwrap();
+    let wormhole = super::wormhole::deploy_with_test_guardian();
+    let fee_contract = deploy_fee_contract(user);
+    let pyth = deploy_default(owner, wormhole.contract_address, fee_contract.contract_address);
+
+    pyth.execute_governance_instruction(data::pyth_set_fee_alt_emitter());
+}
+
+#[test]
+#[should_panic(expected: ('invalid governance data source',))]
+fn test_rejects_old_emitter_after_transfer() {
+    let owner = 'owner'.try_into().unwrap();
+    let user = 'user'.try_into().unwrap();
+    let wormhole = super::wormhole::deploy_with_test_guardian();
+    let fee_contract = deploy_fee_contract(user);
+    let pyth = deploy_default(owner, wormhole.contract_address, fee_contract.contract_address);
+
+    pyth.execute_governance_instruction(data::pyth_auth_transfer());
+    pyth.execute_governance_instruction(data::pyth_set_fee());
+}
+
 fn deploy_default(
     owner: ContractAddress, wormhole_address: ContractAddress, fee_contract_address: ContractAddress
 ) -> IPythDispatcher {

+ 60 - 3
target_chains/starknet/tools/test_vaas/src/bin/generate_test_data.rs

@@ -168,6 +168,9 @@ fn main() {
 
     // Pyth governance payload bytes are copied from
     // `governance/xc_admin/packages/xc_admin_common/src/__tests__/GovernancePayload.test.ts`
+    let pyth_set_fee_payload = vec![
+        80, 84, 71, 77, 1, 3, 234, 147, 0, 0, 0, 0, 0, 0, 0, 42, 0, 0, 0, 0, 0, 0, 0, 2,
+    ];
     let pyth_set_fee = serialize_vaa(guardians.sign_vaa(
         &[0],
         VaaBody {
@@ -177,9 +180,7 @@ fn main() {
             emitter_address: u256_to_be(41.into()).into(),
             sequence: 1.try_into().unwrap(),
             consistency_level: 6,
-            payload: PayloadKind::Binary(vec![
-                80, 84, 71, 77, 1, 3, 234, 147, 0, 0, 0, 0, 0, 0, 0, 42, 0, 0, 0, 0, 0, 0, 0, 2,
-            ]),
+            payload: PayloadKind::Binary(pyth_set_fee_payload.clone()),
         },
     ));
     print_as_cairo_fn(
@@ -233,6 +234,62 @@ fn main() {
         "A Pyth governance instruction to set wormhole address signed by the test guardian #1.",
     );
 
+    let pyth_request_transfer = serialize_vaa(guardians.sign_vaa(
+        &[0],
+        VaaBody {
+            timestamp: 1,
+            nonce: 2,
+            emitter_chain: 2,
+            emitter_address: u256_to_be(43.into()).into(),
+            sequence: 1.try_into().unwrap(),
+            consistency_level: 6,
+            payload: PayloadKind::Binary(vec![80, 84, 71, 77, 1, 5, 234, 147, 0, 0, 0, 1]),
+        },
+    ));
+    print_as_cairo_fn(
+        &pyth_request_transfer,
+        "pyth_request_transfer",
+        "A Pyth governance instruction to request governance data source transfer signed by the test guardian #1.",
+    );
+
+    let mut pyth_auth_transfer_payload = vec![80, 84, 71, 77, 1, 1, 234, 147];
+    pyth_auth_transfer_payload.extend_from_slice(&pyth_request_transfer);
+    let pyth_auth_transfer = serialize_vaa(guardians.sign_vaa(
+        &[0],
+        VaaBody {
+            timestamp: 1,
+            nonce: 2,
+            emitter_chain: 1,
+            emitter_address: u256_to_be(41.into()).into(),
+            sequence: 1.try_into().unwrap(),
+            consistency_level: 6,
+            payload: PayloadKind::Binary(pyth_auth_transfer_payload),
+        },
+    ));
+    print_as_cairo_fn(
+        &pyth_auth_transfer,
+        "pyth_auth_transfer",
+        "A Pyth governance instruction to authorize governance data source transfer signed by the test guardian #1.",
+    );
+
+    let pyth_set_fee_alt_emitter = serialize_vaa(guardians.sign_vaa(
+        &[0],
+        VaaBody {
+            timestamp: 1,
+            nonce: 2,
+            emitter_chain: 2,
+            emitter_address: u256_to_be(43.into()).into(),
+            sequence: 2.try_into().unwrap(),
+            consistency_level: 6,
+            payload: PayloadKind::Binary(pyth_set_fee_payload.clone()),
+        },
+    ));
+    print_as_cairo_fn(
+        &pyth_set_fee_alt_emitter,
+        "pyth_set_fee_alt_emitter",
+        "A Pyth governance instruction to set fee with alternative emitter signed by the test guardian #1.",
+    );
+
     let guardians2 = GuardianSet {
         set_index: 1,
         secrets: vec![SecretKey::parse_slice(&hex::decode(secret2).unwrap()).unwrap()],