fix(sdk): refuse a vote recovery whose proved vote differs from the submitted one

packages/rs-sdk/src/platform/transition/vote.rs

@@ -144,12 +144,27 @@ impl<S: Signer<IdentityPublicKey>> PutVote<S> for Vote {
                 return if e.to_string().contains("already exists") {
                     let vote =
                         Vote::fetch(sdk, VoteQuery::new(voter_pro_tx_hash, vote_poll_id)).await?;
-                    vote.ok_or(Error::Generic(
+                    let vote = vote.ok_or(Error::Generic(
                         "vote was proved to not exist but was said to exist".to_string(),
-                    ))
+                    ))?;
+                    // "Already exists" is only the broadcasting node's word, and a node can give
+                    // it for a changed vote that is not recorded. The proof shows the vote the
+                    // voter holds on this poll, so it confirms the submission only when it is the
+                    // submitted vote: the same equality the regular confirmation checks.
+                    if &vote == self {
+                        Ok(vote)
+                    } else {
+                        let Vote::ResourceVote(proved) = &vote;
+                        Err(Error::InvalidProvedResponse(format!(
+                            "vote was said to exist, but the vote proved on its poll is {}, \
+                             not the submitted {}: the submitted vote is not recorded",
+                            proved.resource_vote_choice(),
+                            resource_vote.resource_vote_choice()
+                        )))
+                    }
                 } else {
                     Err(e.into())
-                }
+                };
             }
         }
         Self::wait_for_response(sdk, masternode_vote_transition, Some(settings)).await
@@ -173,3 +188,185 @@ fn get_voting_identity_id(
         &pub_key_hash,
     ))
 }
+
+#[cfg(all(test, feature = "mocks"))]
+mod tests {
+    use super::*;
+    use crate::platform::transition::masternode_vote_keys::voter_identity_voting_key;
+    use dapi_grpc::tonic::Status;
+    use dpp::address_funds::AddressWitness;
+    use dpp::platform_value::{BinaryData, Value};
+    use dpp::state_transition::StateTransition;
+    use dpp::voting::vote_choices::resource_vote_choice::ResourceVoteChoice;
+    use dpp::voting::vote_polls::contested_document_resource_vote_poll::ContestedDocumentResourceVotePoll;
+    use dpp::voting::vote_polls::VotePoll;
+    use dpp::voting::votes::resource_vote::v0::ResourceVoteV0;
+    use dpp::voting::votes::resource_vote::ResourceVote;
+    use dpp::ProtocolError;
+    use drive_proof_verifier::types::IdentityNonceFetcher;
+    use rs_dapi_client::transport::TransportError;
+    use rs_dapi_client::{CanRetry, DapiClientError, ExecutionError};
+
+    /// Signs with fixed bytes, so the test can rebuild the exact transition the SDK broadcasts.
+    #[derive(Debug)]
+    struct FixedSigner;
+
+    #[async_trait::async_trait]
+    impl Signer<IdentityPublicKey> for FixedSigner {
+        async fn sign(
+            &self,
+            _key: &IdentityPublicKey,
+            _data: &[u8],
+        ) -> Result<BinaryData, ProtocolError> {
+            Ok(BinaryData::new(vec![7; 65]))
+        }
+
+        async fn sign_create_witness(
+            &self,
+            _key: &IdentityPublicKey,
+            _data: &[u8],
+        ) -> Result<AddressWitness, ProtocolError> {
+            Err(ProtocolError::Generic(
+                "a vote has no address witness".to_string(),
+            ))
+        }
+
+        fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool {
+            true
+        }
+    }
+
+    fn pro_tx_hash() -> Identifier {
+        Identifier::new([1; 32])
+    }
+
+    fn voting_key() -> IdentityPublicKey {
+        voter_identity_voting_key(&[2; 20])
+    }
+
+    fn vote_poll() -> VotePoll {
+        VotePoll::ContestedDocumentResourceVotePoll(ContestedDocumentResourceVotePoll {
+            contract_id: Identifier::new([3; 32]),
+            document_type_name: "domain".to_string(),
+            index_name: "parentNameAndLabel".to_string(),
+            index_values: vec![
+                Value::Text("dash".to_string()),
+                Value::Text("alice".to_string()),
+            ],
+        })
+    }
+
+    fn vote(choice: ResourceVoteChoice) -> Vote {
+        Vote::ResourceVote(ResourceVote::V0(ResourceVoteV0 {
+            vote_poll: vote_poll(),
+            resource_vote_choice: choice,
+        }))
+    }
+
+    fn towards(contender: u8) -> ResourceVoteChoice {
+        ResourceVoteChoice::TowardsIdentity(Identifier::new([contender; 32]))
+    }
+
+    /// Submits `submitted` to a node that answers the broadcast with "already exists" and proves
+    /// `recorded` as the voter's vote on the submitted vote's poll.
+    async fn submit_answered_already_exists(
+        submitted: &Vote,
+        recorded: Option<Vote>,
+    ) -> Result<Vote, Error> {
+        let mut sdk = Sdk::new_mock();
+        let voter_identity_id = get_voting_identity_id(pro_tx_hash(), &voting_key()).unwrap();
+        sdk.mock()
+            .expect_fetch(voter_identity_id, Some(IdentityNonceFetcher(10)))
+            .await
+            .unwrap();
+        sdk.mock()
+            .expect_fetch(
+                VoteQuery::new(pro_tx_hash(), submitted.vote_poll_unique_id().unwrap()),
+                recorded,
+            )
+            .await
+            .unwrap();
+
+        // The SDK bumps the fetched nonce once before signing.
+        let transition: StateTransition = MasternodeVoteTransition::try_from_vote_with_signer(
+            submitted.clone(),
+            &FixedSigner,
+            pro_tx_hash(),
+            &voting_key(),
+            11,
+            sdk.version(),
+            None,
+        )
+        .await
+        .unwrap();
+        let broadcast = transition.broadcast_request_for_state_transition().unwrap();
+        sdk.mock_dapi()
+            .lock()
+            .await
+            .expect(
+                &broadcast,
+                &Err(ExecutionError {
+                    inner: DapiClientError::Transport(TransportError::Grpc(
+                        Status::already_exists("state transition already in chain"),
+                    )),
+                    address: None,
+                    retries: 0,
+                }),
+            )
+            .unwrap();
+
+        submitted
+            .put_to_platform_and_wait_for_response(
+                pro_tx_hash(),
+                &voting_key(),
+                &sdk,
+                &FixedSigner,
+                None,
+            )
+            .await
+    }
+
+    /// A node that answers "already exists" to a changed vote must not make the SDK report the
+    /// change as cast: the vote proved on the poll is still the previous choice.
+    #[tokio::test]
+    async fn should_fail_a_vote_change_answered_already_exists_while_the_previous_choice_is_proved()
+    {
+        let recorded = towards(4);
+
+        let result = submit_answered_already_exists(&vote(towards(5)), Some(vote(recorded))).await;
+
+        let error = result.expect_err("the submitted vote is not the one recorded");
+        assert!(
+            matches!(&error, Error::InvalidProvedResponse(message)
+                if message.contains(&recorded.to_string())),
+            "expected a mismatch naming the recorded choice, got {error:?}"
+        );
+        assert!(
+            !error.can_retry(),
+            "the proof is sound, so the mismatch is no node failure to retry past"
+        );
+    }
+
+    /// The duplicate re-broadcast of a vote that was recorded is still confirmed by the proved vote.
+    #[tokio::test]
+    async fn should_confirm_a_vote_answered_already_exists_when_the_same_vote_is_proved() {
+        let submitted = vote(towards(5));
+
+        let confirmed = submit_answered_already_exists(&submitted, Some(submitted.clone()))
+            .await
+            .expect("the recorded vote is the submitted one");
+
+        assert_eq!(confirmed, submitted);
+    }
+
+    /// A vote answered "already exists" whose poll has no proved vote was not recorded.
+    #[tokio::test]
+    async fn should_fail_a_vote_answered_already_exists_when_no_vote_is_proved() {
+        let result = submit_answered_already_exists(&vote(towards(5)), None).await;
+
+        assert!(
+            matches!(result, Err(Error::Generic(_))),
+            "expected the missing vote to be reported, got {result:?}"
+        );
+    }
+}

packages/rs-sdk/src/sdk.rs

@@ -834,6 +834,16 @@ impl Sdk {
         }
     }
 
+    /// The mock DAPI client, for tests that answer a request with an error, which
+    /// [`MockDashPlatformSdk`] expectations cannot express.
+    #[cfg(all(test, feature = "mocks"))]
+    pub(crate) fn mock_dapi(&self) -> &Arc<Mutex<MockDapiClient>> {
+        let SdkInstance::Mock { dapi, .. } = &self.inner else {
+            panic!("not a mock")
+        };
+        dapi
+    }
+
     /// Get or fetch identity nonce, querying Platform when stale or absent.
     /// Treats a missing nonce as `0` before applying the optional bump; on first
     /// interaction this may return `0` or `1` depending on `bump_first`. Does not

packages/rs-sdk/src/sdk/protocol_gate_tests.rs

@@ -8,7 +8,7 @@
 use super::document_query_shape_tests::CheckedAt;
 use super::identity_discovery_tests as discovery;
 use super::protocol_gate::{decide, version_for_requests, Gate, GateState};
-use super::{Sdk, SdkInstance};
+use super::Sdk;
 use crate::error::{Error, StaleNodeError};
 use crate::platform::contract_fee_pots::ContractFeePots;
 use crate::platform::contract_groups::{
@@ -78,15 +78,13 @@ use drive_proof_verifier::types::{
     Documents, MostRecentShieldedAnchor, ShieldedAnchors, ShieldedEncryptedNotes,
     ShieldedNotesCount, ShieldedNullifierStatuses, ShieldedPoolState,
 };
-use rs_dapi_client::mock::MockDapiClient;
 use rs_dapi_client::transport::{TransportError, TransportRequest};
 use rs_dapi_client::{
     Address, DapiClientError, ExecutionError, ExecutionResponse, RequestSettings,
 };
 use std::path::PathBuf;
 use std::sync::atomic::Ordering;
 use std::sync::Arc;
-use tokio::sync::Mutex;
 
 /// The protocol version a later protocol upgrade's requests need.
 const V14: u32 = PROTOCOL_VERSION_14;
@@ -210,13 +208,6 @@ fn sdk_with_provider<C: ContextProvider + 'static>(
     sdk
 }
 
-fn dapi(sdk: &Sdk) -> &Arc<Mutex<MockDapiClient>> {
-    let SdkInstance::Mock { dapi, .. } = &sdk.inner else {
-        panic!("mock")
-    };
-    dapi
-}
-
 /// The node answers `request` with `answer`.
 async fn answer<R>(sdk: &Sdk, request: &R, answer: Result<R::Response, Status>)
 where
@@ -235,12 +226,16 @@ where
             retries: 0,
         }),
     };
-    dapi(sdk).lock().await.expect(request, &result).unwrap();
+    sdk.mock_dapi()
+        .lock()
+        .await
+        .expect(request, &result)
+        .unwrap();
 }
 
 /// How many times `request` reached the node.
 async fn sent<R: Mockable>(sdk: &Sdk, request: &R) -> usize {
-    dapi(sdk).lock().await.request_count(request)
+    sdk.mock_dapi().lock().await.request_count(request)
 }
 
 /// The genesis epoch, which the SDK asks for to learn the network's protocol version.