chore: index nodes by account id & TLS key in TEE State by kevindeforth · Pull Request #1120 · near/mpc

kevindeforth · 2025-09-16T12:07:04Z

Introduces NodeUid as a unique identifier for participants nodes.

pub struct NodeUid {
    /// Operator account
    pub account_id: AccountId,
    /// TLS public key
    pub tls_public_key: PublicKey,
}

Reasonable assumptions about the network state are:

Each node operator is uniquely identified by their AccountId (existing assumption).
A node operator may only have one participating node at any point in time (enforced by the Participants struct).
A node operator may have multiple nodes submitting an attestation to the contract (enabled by this PR);
Each node is uniquely identified by their tls_public_key (not really enforceable on the contract).

Note that the following give probabilistic guarantees for point 4, if nodes run inside TEEs:

a new node must generate a new TLS key upon startup;
the TLS key is measured and included in the attestation.

Uncovered issues:

DSharifi · 2025-09-17T08:12:12Z

+    pub fn get_node_uids(&self) -> BTreeSet<NodeUid> {
+        self.participants()
+            .iter()
+            .map(|(account_id, _, p_info)| NodeUid {
+                account_id: account_id.clone(),
+                tls_public_key: p_info.sign_pk.clone(),
+            })
+            .collect()
+    }


We don't "need" to have this test utility in the source code as part of a test-utils feature. Nothing here relies on private attributes or methods.

So we can define this impl extension or as a function in our test utilties, for example in common.rs.

netrome

Looks good to me. Nice stuff! Got a test failure though.

kevindeforth · 2025-09-17T14:20:54Z

-/// TEE accounts for accounts that are no longer in the participant list.
-/// This simulates cleanup after participant removal (e.g., post-resharing).
-#[test]
-fn test_clean_tee_status_removes_non_participants() {


Can be removed, this is almost a duplicate of the one in tee.rs

mpc/libs/chain-signatures/contract/tests/tee.rs

Line 335 in 95649d0

async fn test_clean_tee_status_succeeds_when_contract_calls_itself() -> Result<()> {

👍🏼

Also, it was discussed offline: https://nearone.slack.com/archives/C0912BTG51T/p1758033418475589.

netrome

I'd like to keep using testing_env! over the sandbox environment for the kickout and cleanup tests.

netrome · 2025-09-17T14:27:28Z

                }

-                Ok(false)
+                Ok(true)


Nice. I assume we have test coverage for this bug now but didn't in the past? Or did you just notice this on the fly?

Makes you wonder what the point of these return values are if we never get them back anyway on the node :')

We do have test coverage now, yes. But didn't before.
Edit: in fact, we tested the wrong behavior:

mpc/crates/contract/tests/expired_attestation.rs

Line 136 in 77e6de4

assert!(!setup.contract.verify_tee().unwrap());

I mean that the return value will never be observed by the node who is responsible for calling this method. So the return value doesn't serve any purpose.

I mean that the return value will never be observed by the node who is responsible for calling this method. So the return value doesn't serve any purpose.

Hmm, IMO, the fact that the node doesn't track the transaction outcome does not invalidate that our methods return values. They can still be inspected in our testing frameworks and are observable through manual inspection on Nearblocks

But I do think that it would be much desired for our nodes to track return values as well.

DSharifi

The tests in expired_attestation.rs need to be kept. They are not the same as the one in tee.rs

The one in tee.rs tests that the [private] attribute is set, I.E. only the contract can call the methods.

The tests in expired_attestation.rs were just added today. They test the flow of cleanups after resharing work as a result of resharing through expired attestations.

CC: @pbeza as he has more context as author of these tests

pbeza · 2025-09-17T14:49:22Z

CC: @Patrick1904 as he has more context as author of these tests

@DSharifi, my GitHub username is @pbeza. :)

The tests in expired_attestation.rs need to be kept. They are not the same as the one in tee.rs

The one in tee.rs tests that the [private] attribute is set, I.E. only the contract can call the methods.

The tests in expired_attestation.rs were just added today. They test the flow of cleanups after resharing work as a result of resharing through expired attestations.

I agree with @DSharifi, nothing much to add. From my perspective, these are two different tests (though somewhat similar, especially test_clean_tee_status_removes_non_participants) covering TEE cleanup from different angles, as Daniel described, so ideally I’d keep both. I don’t have the full context on why we want to drop one of them—just because they look similar?

kevindeforth · 2025-09-17T14:59:04Z

The tests in expired_attestation.rs need to be kept. They are not the same as the one in tee.rs

We keep this one:

mpc/libs/chain-signatures/contract/tests/expired_attestation.rs

Lines 74 to 176 in 95649d0

    
           /// **Integration test for participant kickout after expiration** - Tests expired attestation removal. This test demonstrates the complete kickout mechanism using direct contract calls: 
        
           /// 1. Initialize contract with 3 secp256k1 participants in Running state at time T=1s 
        
           /// 2. Submit valid attestations for first 2 participants at time T=1s 
        
           /// 3. Submit expiring attestation for 3rd participant with expiry at time T+10s 
        
           /// 4. Fast-forward blockchain time to T+20s using VMContextBuilder 
        
           /// 5. Call verify_tee() which detects expired attestation and returns false 
        
           /// 6. Contract automatically transitions from Running to Resharing state 
        
           /// 7. Start resharing instance and have valid participants vote for completion 
        
           /// 8. Contract transitions back to Running state with filtered participant set (2 participants) 
        
           /// 9. Verify final state: 2 participants in Running state but 3 TEE accounts remain (cleanup tested separately) 
        
           #[test] 
        
           fn test_participant_kickout_after_expiration() { 
        
               const INITIAL_TIME_SECONDS: u64 = 1; 
        
               const INITIAL_TIMESTAMP_NANOS: u64 = 
        
                   Duration::from_secs(INITIAL_TIME_SECONDS).as_nanos() as u64; 
        
               const PARTICIPANT_COUNT: usize = 3; 
        
               const THRESHOLD: u64 = 2; 
        
               const EXPIRY_OFFSET_SECONDS: u64 = 10; // Attestation expires 10 seconds after start 
        
               const POST_EXPIRY_WAIT_SECONDS: u64 = 20; // Wait 20 seconds after start to trigger resharing 
        
               testing_env!(VMContextBuilder::new() 
        
                   .block_timestamp(INITIAL_TIMESTAMP_NANOS) 
        
                   .attached_deposit(NearToken::from_near(1)) 
        
                   .build()); 
        
               let domain_id = DomainId::default(); 
        
               let mut setup = TestSetup::new(PARTICIPANT_COUNT, THRESHOLD); 
        
               assert_eq!(setup.contract.get_tee_accounts().len(), 0); 
        
               // Submit valid attestations for first 2 participants 
        
               let valid_attestation = Attestation::Mock(MockAttestation::Valid); 
        
               let participant_accounts: Vec<AccountId> = setup 
        
                   .participants_list 
        
                   .iter() 
        
                   .take(2) 
        
                   .map(|(account_id, _, _)| account_id.clone()) 
        
                   .collect(); 
        
               for account_id in &participant_accounts { 
        
                   setup.submit_attestation_for_participant(account_id, valid_attestation.clone()); 
        
               } 
        
               // Submit expiring attestation for 3rd participant 
        
               const EXPIRY_SECONDS: u64 = INITIAL_TIME_SECONDS + EXPIRY_OFFSET_SECONDS; 
        
               let expiring_attestation = Attestation::Mock(MockAttestation::WithConstraints { 
        
                   mpc_docker_image_hash: None, 
        
                   launcher_docker_compose_hash: None, 
        
                   expiry_time_stamp_seconds: Some(EXPIRY_SECONDS), 
        
               }); 
        
               let third_participant = setup.participants_list[2].0.clone(); 
        
               setup.submit_attestation_for_participant(&third_participant, expiring_attestation); 
        
               assert_eq!(setup.contract.get_tee_accounts().len(), PARTICIPANT_COUNT); 
        
               // Fast-forward time past expiry and trigger resharing 
        
               const EXPIRED_TIMESTAMP: u64 = 
        
                   INITIAL_TIMESTAMP_NANOS + Duration::from_secs(POST_EXPIRY_WAIT_SECONDS).as_nanos() as u64; 
        
               testing_env!(VMContextBuilder::new() 
        
                   .block_timestamp(EXPIRED_TIMESTAMP) 
        
                   .build()); 
        
               assert!(!setup.contract.verify_tee().unwrap()); 
        
               let resharing_state = match setup.contract.state() { 
        
                   ProtocolContractState::Resharing(r) => r, 
        
                   state => panic!("Should be in Resharing state. Actual state {:#?}", state), 
        
               }; 
        
               // Complete resharing process 
        
               let key_event_id = KeyEventId::new( 
        
                   resharing_state.prospective_epoch_id(), 
        
                   domain_id, 
        
                   AttemptId::new(), 
        
               ); 
        
               testing_env!(create_context_for_participant(&participant_accounts[0])); 
        
               setup.contract.start_reshare_instance(key_event_id).unwrap(); 
        
               // Vote for resharing with first 2 participants 
        
               for account_id in &participant_accounts { 
        
                   testing_env!(create_context_for_participant(account_id)); 
        
                   setup.contract.vote_reshared(key_event_id).unwrap(); 
        
               } 
        
               // Verify final state: back to Running with one less participant 
        
               assert_matches!(setup.contract.state(), ProtocolContractState::Running(_)); 
        
               // At this point we have 2 participants in Running state but 3 TEE accounts 
        
               // The cleanup step is tested separately in test_clean_tee_status_removes_non_participants 
        
               let final_running_state = match setup.contract.state() { 
        
                   ProtocolContractState::Running(r) => r, 
        
                   _ => panic!("Should be in Running state after resharing"), 
        
               }; 
        
               const EXPECTED_PARTICIPANT_COUNT: usize = PARTICIPANT_COUNT - 1; 
        
               assert_eq!( 
        
                   final_running_state.parameters.participants().len(), 
        
                   EXPECTED_PARTICIPANT_COUNT 
        
               ); 
        
               // Before clean_tee_status() cleanup, we still have old TEE accounts 
        
               assert_eq!(setup.contract.get_tee_accounts().len(), PARTICIPANT_COUNT); 
        
           }

but we remove

mpc/libs/chain-signatures/contract/tests/expired_attestation.rs

Lines 178 to 241 in 95649d0

    
           /// **Unit test for TEE cleanup of non-participants** - Tests that `clean_tee_status()` removes 
        
           /// TEE accounts for accounts that are no longer in the participant list. 
        
           /// This simulates cleanup after participant removal (e.g., post-resharing). 
        
           #[test] 
        
           fn test_clean_tee_status_removes_non_participants() { 
        
               const PARTICIPANT_COUNT: usize = 2; // After resharing removed one participant 
        
               const THRESHOLD: u64 = 2; 
        
               testing_env!(VMContextBuilder::new() 
        
                   .attached_deposit(NearToken::from_near(1)) 
        
                   .build()); 
        
               // Create contract in Running state with 2 current participants 
        
               let mut setup = TestSetup::new(PARTICIPANT_COUNT, THRESHOLD); 
        
               // Submit TEE info for current 2 participants (all have valid attestations) 
        
               let valid_attestation = Attestation::Mock(MockAttestation::Valid); 
        
               let participant_accounts: Vec<AccountId> = setup 
        
                   .participants_list 
        
                   .iter() 
        
                   .map(|(account_id, _, _)| account_id.clone()) 
        
                   .collect(); 
        
               for account_id in &participant_accounts { 
        
                   setup.submit_attestation_for_participant(account_id, valid_attestation.clone()); 
        
               } 
        
               // Add TEE account for someone who is NOT a current participant 
        
               // (simulates leftover data from a participant who was removed during resharing) 
        
               let removed_participant_id: AccountId = "removed.participant.near".parse().unwrap(); 
        
               setup.submit_attestation_for_participant(&removed_participant_id, valid_attestation); 
        
               // Verify initial state: 2 participants but 3 TEE accounts 
        
               const INITIAL_TEE_ACCOUNTS: usize = PARTICIPANT_COUNT + 1; // 2 current + 1 stale 
        
               assert_eq!( 
        
                   setup.contract.get_tee_accounts().len(), 
        
                   INITIAL_TEE_ACCOUNTS 
        
               ); 
        
               let running_state = match setup.contract.state() { 
        
                   ProtocolContractState::Running(r) => r, 
        
                   _ => panic!("Should be in Running state"), 
        
               }; 
        
               assert_eq!( 
        
                   running_state.parameters.participants().len(), 
        
                   PARTICIPANT_COUNT 
        
               ); 
        
               // Test cleanup: should remove TEE account for non-participant 
        
               setup.contract.clean_tee_status().unwrap(); 
        
               // Verify cleanup worked: TEE accounts reduced to match participant count 
        
               assert_eq!(setup.contract.get_tee_accounts().len(), PARTICIPANT_COUNT); 
        
               // State should remain Running with same participant count 
        
               let final_running_state = match setup.contract.state() { 
        
                   ProtocolContractState::Running(r) => r, 
        
                   _ => panic!("Should still be Running after cleanup"), 
        
               }; 
        
               assert_eq!( 
        
                   final_running_state.parameters.participants().len(), 
        
                   PARTICIPANT_COUNT 
        
               ); 
        
           }

which tests the same thing as

mpc/libs/chain-signatures/contract/tests/tee.rs

Lines 331 to 392 in 95649d0

    
           /// **TEE cleanup functionality** - Tests that the clean_tee_status contract method works correctly when called by the contract itself. 
        
           /// Unlike the access control test above, this demonstrates the positive case: the contract can successfully clean up 
        
           /// TEE data for accounts that are no longer participants. Uses the test method to populate initial TEE state. 
        
           #[tokio::test] 
        
           async fn test_clean_tee_status_succeeds_when_contract_calls_itself() -> Result<()> { 
        
               let (worker, contract, accounts, _) = init_env_secp256k1(1).await; 
        
               // Initially should have no TEE participants 
        
               assert_eq!(get_tee_accounts(&contract).await?.len(), 0); 
        
               // Setup contract with approved hash and submit TEE info for current participants 
        
               setup_approved_mpc_hash(&contract, &accounts).await?; 
        
               let tls_key = p2p_tls_key(); 
        
               let attestation = mock_dstack_attestation(); 
        
               for account in &accounts { 
        
                   let success = submit_participant_info(account, &contract, &attestation, &tls_key).await?; 
        
                   assert!(success); 
        
               } 
        
               // Verify current participants have TEE data 
        
               assert_eq!(get_tee_accounts(&contract).await?.len(), accounts.len()); 
        
               // Create additional accounts (non-participants) and submit TEE info for them 
        
               const NUM_ADDITIONAL_ACCOUNTS: usize = 2; 
        
               let additional_accounts = gen_accounts(&worker, NUM_ADDITIONAL_ACCOUNTS).await.0; 
        
               for account in &additional_accounts { 
        
                   let success = submit_participant_info(account, &contract, &attestation, &tls_key).await?; 
        
                   assert!(success); 
        
               } 
        
               // Verify we have TEE data for all accounts before cleanup 
        
               let tee_participants_before = get_tee_accounts(&contract).await?; 
        
               assert_eq!( 
        
                   tee_participants_before.len(), 
        
                   accounts.len() + additional_accounts.len() 
        
               ); 
        
               // Contract should be able to call clean_tee_status on itself 
        
               let result = contract 
        
                   .as_account() 
        
                   .call(contract.id(), "clean_tee_status") 
        
                   .args_json(serde_json::json!({})) 
        
                   .transact() 
        
                   .await?; 
        
               assert!(result.is_success()); 
        
               // Verify cleanup worked: only current participants should have TEE data 
        
               let tee_participants_after = get_tee_accounts(&contract).await?; 
        
               assert_eq!(tee_participants_after.len(), accounts.len()); 
        
               let expected_participants: HashSet<_> = accounts 
        
                   .iter() 
        
                   .map(|account| account.id().clone()) 
        
                   .collect(); 
        
               let actual_participants: HashSet<_> = tee_participants_after.into_iter().collect(); 
        
               assert_eq!(expected_participants, actual_participants); 
        
               Ok(()) 
        
           }

The one in tee.rs tests that the [private] attribute is set, I.E. only the contract can call the methods.

Yes, but that is a different test to the one we are discussing and irrelevant for this PR. You are referring to this test here:

mpc/libs/chain-signatures/contract/tests/tee.rs

Lines 300 to 329 in 95649d0

    
           /// **Access control validation** - Tests that external accounts cannot call the private clean_tee_status contract method. 
        
           /// This verifies the security boundary: only the contract itself should be able to perform internal cleanup operations. 
        
           #[tokio::test] 
        
           async fn test_clean_tee_status_denies_external_account_access() -> Result<()> { 
        
               let (worker, contract, _accounts, _) = init_env_secp256k1(1).await; 
        
               // Create a new account that's not the contract 
        
               let external_account = worker.dev_create_account().await?; 
        
               // Try to call clean_tee_status from external account - should fail 
        
               let result = external_account 
        
                   .call(contract.id(), "clean_tee_status") 
        
                   .args_json(serde_json::json!({})) 
        
                   .transact() 
        
                   .await?; 
        
               // The call should fail because it's not from the contract itself 
        
               assert!(!result.is_success()); 
        
               // Verify the error message indicates unauthorized access 
        
               match result.into_result() { 
        
                   Err(failure) => { 
        
                       let error_msg = format!("{:?}", failure); 
        
                       assert!(error_msg.contains("Method clean_tee_status is private")); 
        
                   } 
        
                   Ok(_) => panic!("Call should have failed"), 
        
               } 
        
               Ok(()) 
        
           }

The tests in expired_attestation.rs were just added today. They test the flow of cleanups after resharing work as a result of resharing through expired attestations.

Not exactly. The tests in expired_attesation.rs test that the contract enters a resharing state if verify_tee is called while an existing participant has an expired attestation.

Edit:
in fact, because this is not a sandbox best, we can't test that in expired_attestation c.f. https://github.com/near/mpc/blob/95649d0982a98942d6249e4d7efaf0758d0155b4/libs/chain-signatures/contract/tests/expired_attestation.rs#L175C57-L175C76

pbeza · 2025-09-17T18:44:16Z

OK, I re-read the implementation of both:

mpc/libs/chain-signatures/contract/tests/expired_attestation.rs

Lines 178 to 241 in 95649d0

    
           /// **Unit test for TEE cleanup of non-participants** - Tests that `clean_tee_status()` removes 
        
           /// TEE accounts for accounts that are no longer in the participant list. 
        
           /// This simulates cleanup after participant removal (e.g., post-resharing). 
        
           #[test] 
        
           fn test_clean_tee_status_removes_non_participants() { 
        
               const PARTICIPANT_COUNT: usize = 2; // After resharing removed one participant 
        
               const THRESHOLD: u64 = 2; 
        
               testing_env!(VMContextBuilder::new() 
        
                   .attached_deposit(NearToken::from_near(1)) 
        
                   .build()); 
        
               // Create contract in Running state with 2 current participants 
        
               let mut setup = TestSetup::new(PARTICIPANT_COUNT, THRESHOLD); 
        
               // Submit TEE info for current 2 participants (all have valid attestations) 
        
               let valid_attestation = Attestation::Mock(MockAttestation::Valid); 
        
               let participant_accounts: Vec<AccountId> = setup 
        
                   .participants_list 
        
                   .iter() 
        
                   .map(|(account_id, _, _)| account_id.clone()) 
        
                   .collect(); 
        
               for account_id in &participant_accounts { 
        
                   setup.submit_attestation_for_participant(account_id, valid_attestation.clone()); 
        
               } 
        
               // Add TEE account for someone who is NOT a current participant 
        
               // (simulates leftover data from a participant who was removed during resharing) 
        
               let removed_participant_id: AccountId = "removed.participant.near".parse().unwrap(); 
        
               setup.submit_attestation_for_participant(&removed_participant_id, valid_attestation); 
        
               // Verify initial state: 2 participants but 3 TEE accounts 
        
               const INITIAL_TEE_ACCOUNTS: usize = PARTICIPANT_COUNT + 1; // 2 current + 1 stale 
        
               assert_eq!( 
        
                   setup.contract.get_tee_accounts().len(), 
        
                   INITIAL_TEE_ACCOUNTS 
        
               ); 
        
               let running_state = match setup.contract.state() { 
        
                   ProtocolContractState::Running(r) => r, 
        
                   _ => panic!("Should be in Running state"), 
        
               }; 
        
               assert_eq!( 
        
                   running_state.parameters.participants().len(), 
        
                   PARTICIPANT_COUNT 
        
               ); 
        
               // Test cleanup: should remove TEE account for non-participant 
        
               setup.contract.clean_tee_status().unwrap(); 
        
               // Verify cleanup worked: TEE accounts reduced to match participant count 
        
               assert_eq!(setup.contract.get_tee_accounts().len(), PARTICIPANT_COUNT); 
        
               // State should remain Running with same participant count 
        
               let final_running_state = match setup.contract.state() { 
        
                   ProtocolContractState::Running(r) => r, 
        
                   _ => panic!("Should still be Running after cleanup"), 
        
               }; 
        
               assert_eq!( 
        
                   final_running_state.parameters.participants().len(), 
        
                   PARTICIPANT_COUNT 
        
               ); 
        
           }

mpc/libs/chain-signatures/contract/tests/tee.rs

Lines 331 to 392 in 95649d0

    
           /// **TEE cleanup functionality** - Tests that the clean_tee_status contract method works correctly when called by the contract itself. 
        
           /// Unlike the access control test above, this demonstrates the positive case: the contract can successfully clean up 
        
           /// TEE data for accounts that are no longer participants. Uses the test method to populate initial TEE state. 
        
           #[tokio::test] 
        
           async fn test_clean_tee_status_succeeds_when_contract_calls_itself() -> Result<()> { 
        
               let (worker, contract, accounts, _) = init_env_secp256k1(1).await; 
        
               // Initially should have no TEE participants 
        
               assert_eq!(get_tee_accounts(&contract).await?.len(), 0); 
        
               // Setup contract with approved hash and submit TEE info for current participants 
        
               setup_approved_mpc_hash(&contract, &accounts).await?; 
        
               let tls_key = p2p_tls_key(); 
        
               let attestation = mock_dstack_attestation(); 
        
               for account in &accounts { 
        
                   let success = submit_participant_info(account, &contract, &attestation, &tls_key).await?; 
        
                   assert!(success); 
        
               } 
        
               // Verify current participants have TEE data 
        
               assert_eq!(get_tee_accounts(&contract).await?.len(), accounts.len()); 
        
               // Create additional accounts (non-participants) and submit TEE info for them 
        
               const NUM_ADDITIONAL_ACCOUNTS: usize = 2; 
        
               let additional_accounts = gen_accounts(&worker, NUM_ADDITIONAL_ACCOUNTS).await.0; 
        
               for account in &additional_accounts { 
        
                   let success = submit_participant_info(account, &contract, &attestation, &tls_key).await?; 
        
                   assert!(success); 
        
               } 
        
               // Verify we have TEE data for all accounts before cleanup 
        
               let tee_participants_before = get_tee_accounts(&contract).await?; 
        
               assert_eq!( 
        
                   tee_participants_before.len(), 
        
                   accounts.len() + additional_accounts.len() 
        
               ); 
        
               // Contract should be able to call clean_tee_status on itself 
        
               let result = contract 
        
                   .as_account() 
        
                   .call(contract.id(), "clean_tee_status") 
        
                   .args_json(serde_json::json!({})) 
        
                   .transact() 
        
                   .await?; 
        
               assert!(result.is_success()); 
        
               // Verify cleanup worked: only current participants should have TEE data 
        
               let tee_participants_after = get_tee_accounts(&contract).await?; 
        
               assert_eq!(tee_participants_after.len(), accounts.len()); 
        
               let expected_participants: HashSet<_> = accounts 
        
                   .iter() 
        
                   .map(|account| account.id().clone()) 
        
                   .collect(); 
        
               let actual_participants: HashSet<_> = tee_participants_after.into_iter().collect(); 
        
               assert_eq!(expected_participants, actual_participants); 
        
               Ok(()) 
        
           }

These tests are quite similar conceptually (with some minor differences, like the number of participants and checking that only the contract can call the clean_tee_status method), but they use different test primitives (VMContext vs sandbox), which is already enough reason for me to keep both.

I’m not sure I follow why there’s so much insistence on removing one of them (sorry, I haven’t spent too much time looking into the context of this PR yet), but from my perspective test_clean_tee_status_removes_non_participants is probably less important than test_participant_kickout_after_expiration. So if you want to drop that one, I won’t oppose too strongly.

I think this discussion might never have started if I hadn’t followed this suggestion, where I refactored a single test into two separate ones—one of which now looks very similar to the one in tee.rs. Before I addressed that comment, the test we’re now debating whether to remove was essentially just one extra assert! statement:

mpc/libs/chain-signatures/contract/tests/expired_attestation.rs

Lines 134 to 146 in 68a67cc

    
           contract.clean_tee_status().unwrap(); 
        
           let final_running_state = match contract.state() { 
        
               ProtocolContractState::Running(r) => r, 
        
               _ => panic!("Should still be Running after cleanup"), 
        
           }; 
        
           const EXPECTED_COUNT: usize = PARTICIPANT_COUNT - 1; 
        
           assert_eq!( 
        
               final_running_state.parameters.participants().len(), 
        
               EXPECTED_COUNT 
        
           ); 
        
           assert_eq!(contract.get_tee_accounts().len(), EXPECTED_COUNT);

which you might not even have noticed, so it wouldn’t have raised any suspicion. :P

Also, note that I had already pointed out in this comment, before this whole discussion started, that test_clean_tee_status_removes_non_participants is indeed very similar to the existing one in tee.rs.

netrome

Thank you for updating 🙏 ! Would be great if we could get this through now.

netrome · 2025-09-18T11:13:21Z

        .build());

-    assert!(!setup.contract.verify_tee().unwrap());
+    assert!(setup.contract.verify_tee().unwrap());


Hmm flipping this assertion does reduce what we're able to test here. As we discussed, we should extend the return value of verify_tee to convey whether or not all attestations are valid. However, we can do that later since this is technically a bug fix in the verify_tee method.

kevindeforth force-pushed the kd/1084-migration-service branch 2 times, most recently from 20bc1a7 to c972361 Compare September 16, 2025 14:06

kevindeforth marked this pull request as ready for review September 16, 2025 14:15

kevindeforth requested review from netrome and pbeza September 16, 2025 14:19

DSharifi reviewed Sep 17, 2025

View reviewed changes

netrome reviewed Sep 17, 2025

View reviewed changes

Comment thread libs/chain-signatures/contract/tests/common.rs Outdated

kevindeforth force-pushed the kd/1084-migration-service branch from 6dbad1a to 3d13120 Compare September 17, 2025 12:36

netrome approved these changes Sep 17, 2025

View reviewed changes

DSharifi approved these changes Sep 17, 2025

View reviewed changes

netrome mentioned this pull request Sep 17, 2025

chore: remove enum VersionedMpcContract contract state in favor of a single version struct #1111

Merged

kevindeforth commented Sep 17, 2025

View reviewed changes

netrome requested changes Sep 17, 2025

View reviewed changes

DSharifi requested changes Sep 17, 2025

View reviewed changes

kevindeforth mentioned this pull request Sep 18, 2025

Fix contract integration tests #1144

Closed

kevindeforth force-pushed the kd/1084-migration-service branch from b44643a to 2c51fd8 Compare September 18, 2025 09:51

kevindeforth and others added 9 commits September 18, 2025 12:00

introduce NodeUid in Tee state

fdb3ac1

cleanup tests

c3bd06e

function order

bd9c4f0

sorting

9366758

require deposit for additional attestations

e371524

cleanup

3fdfa7e

rename NodeUid to NodeId

0b856bd

fix: Update new test cases

6753316

fix #1145

8ca6f35

kevindeforth force-pushed the kd/1084-migration-service branch from 2c51fd8 to 8ca6f35 Compare September 18, 2025 10:02

clippy

abcf68d

use cmp to avoid clone

486ac3e

netrome approved these changes Sep 18, 2025

View reviewed changes

DSharifi approved these changes Sep 18, 2025

View reviewed changes

netrome mentioned this pull request Sep 18, 2025

Fix verify_tee return value #1145

Closed

netrome linked an issue Sep 18, 2025 that may be closed by this pull request

Fix verify_tee return value #1145

Closed

kevindeforth added this pull request to the merge queue Sep 18, 2025

Merged via the queue into main with commit 79a0f2f Sep 18, 2025
9 checks passed

kevindeforth deleted the kd/1084-migration-service branch September 18, 2025 12:08

DSharifi mentioned this pull request Sep 21, 2025

Fix contract integration test TLS key #1119

Closed

Conversation

kevindeforth commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DSharifi Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

netrome left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevindeforth Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

pbeza Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netrome left a comment

Choose a reason for hiding this comment

Uh oh!

netrome Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

DSharifi Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

kevindeforth Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DSharifi Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

kevindeforth Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DSharifi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbeza commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevindeforth commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pbeza commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netrome left a comment

Choose a reason for hiding this comment

Uh oh!

netrome Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kevindeforth commented Sep 16, 2025 •

edited

Loading

netrome left a comment •

edited

Loading

pbeza Sep 18, 2025 •

edited

Loading

kevindeforth Sep 17, 2025 •

edited

Loading

DSharifi left a comment •

edited

Loading

pbeza commented Sep 17, 2025 •

edited

Loading

kevindeforth commented Sep 17, 2025 •

edited

Loading

pbeza commented Sep 17, 2025 •

edited

Loading