Fix req to recourse for payer is holder situations (#694)

Author: zupzup

CHANGELOG.md

@@ -1,6 +1,7 @@
 # 0.4.12
 
 * Added `actions` to `BitcreditBillResult`, with `bill_actions`, that are calculated based on which bill actions the caller is currently allowed to do (breaking DB and API change)
+* Fix an edge case for request to recourse if the payer == holder - they should not have past endorsees and if the payer is a contingent holder, they should not show up in past endorsees
 
 # 0.4.11
 

crates/bcr-ebill-core/src/bill/validation.rs

@@ -683,7 +683,8 @@ mod tests {
         country::Country,
         tests::tests::{
             OTHER_VALID_PAYMENT_ADDRESS_TESTNET, VALID_PAYMENT_ADDRESS_TESTNET, bill_id_test,
-            node_id_test, node_id_test_other, private_key_test, safe_deadline_ts, valid_address,
+            node_id_test, node_id_test_and_another, node_id_test_other, private_key_test,
+            safe_deadline_ts, valid_address, valid_and_another_bill_identified_participant,
             valid_bill_identified_participant, valid_bill_participant,
             valid_other_bill_identified_participant, valid_other_bill_participant,
         },
@@ -1149,9 +1150,9 @@ mod tests {
     }
 
     #[rstest]
-    #[case::req_to_recourse_not_rejected_but_expired(BillValidateActionData { signer_node_id: node_id_test_other(), timestamp: now().timestamp() as u64 + (RECOURSE_DEADLINE_SECONDS * 2), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
-    #[case::req_to_recourse_not_expired_but_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_accept_block(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())))) }, Ok(()))]
-    #[case::req_to_recourse_not_req_to_accept_but_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
+    #[case::req_to_recourse_not_rejected_but_expired(BillValidateActionData { signer_node_id: node_id_test_and_another(), timestamp: now().timestamp() as u64 + (RECOURSE_DEADLINE_SECONDS * 2), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
+    #[case::req_to_recourse_not_expired_but_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_accept_block(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant())))) }, Ok(()))]
+    #[case::req_to_recourse_not_req_to_accept_but_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
     fn test_validate_bill_req_to_recourse_accept_valid(
         #[case] input: BillValidateActionData,
         #[case] expected: Result<(), ValidationError>,
@@ -1166,8 +1167,8 @@ mod tests {
     #[case::active_recourse_blocked(BillValidateActionData { mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_recourse_accept_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),))) }, Err(ValidationError::BillIsInRecourseAndWaitingForPayment))]
     #[case::req_to_recourse_not_holder(BillValidateActionData { mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), signer_node_id: node_id_test(), ..valid_bill_validate_action_data(valid_bill_blockchain_issue( valid_bill_issue_block_data(),)) }, Err(ValidationError::CallerIsNotHolder))]
     #[case::req_to_recourse_not_past_endorsee(BillValidateActionData { endorsee_node_id: Some(node_id_test()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_other_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::RecourseeNotPastHolder))]
-    #[case::req_to_recourse_not_req_to_accept_or_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::BillRequestToAcceptDidNotExpireAndWasNotRejected))]
-    #[case::req_to_recourse_not_expired_or_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()))) }, Err(ValidationError::BillRequestToAcceptDidNotExpireAndWasNotRejected))]
+    #[case::req_to_recourse_not_req_to_accept_or_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::BillRequestToAcceptDidNotExpireAndWasNotRejected))]
+    #[case::req_to_recourse_not_expired_or_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Accept, safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_accept_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant()))) }, Err(ValidationError::BillRequestToAcceptDidNotExpireAndWasNotRejected))]
     fn test_validate_bill_req_to_recourse_accept_errors(
         #[case] input: BillValidateActionData,
         #[case] expected: Result<(), ValidationError>,
@@ -1176,8 +1177,8 @@ mod tests {
     }
 
     #[rstest]
-    #[case::req_to_recourse_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_pay_block(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())))) }, Ok(()))]
-    #[case::req_to_recourse_expired(BillValidateActionData { signer_node_id: node_id_test_other(), timestamp: now().timestamp() as u64 + (RECOURSE_DEADLINE_SECONDS * 3), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
+    #[case::req_to_recourse_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_reject_pay_block(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant())))) }, Ok(()))]
+    #[case::req_to_recourse_expired(BillValidateActionData { signer_node_id: node_id_test_and_another(), timestamp: now().timestamp() as u64 + (RECOURSE_DEADLINE_SECONDS * 3), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant()))) }, Ok(()))]
     fn test_validate_bill_req_to_recourse_payment_valid(
         #[case] input: BillValidateActionData,
         #[case] expected: Result<(), ValidationError>,
@@ -1193,8 +1194,8 @@ mod tests {
     #[case::req_to_recourse_not_holder(BillValidateActionData { mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), signer_node_id: node_id_test(), ..valid_bill_validate_action_data(valid_bill_blockchain_issue( valid_bill_issue_block_data(),)) }, Err(ValidationError::CallerIsNotHolder))]
     #[case::req_to_recourse_not_past_endorsee(BillValidateActionData { endorsee_node_id: Some(node_id_test()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_other_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::RecourseeNotPastHolder))]
     #[case::req_to_recourse_paid(BillValidateActionData { is_paid: true, endorsee_node_id: Some(node_id_test()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_other_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()), valid_bill_identified_participant(), valid_other_bill_identified_participant())) }, Err(ValidationError::BillAlreadyPaid))]
-    #[case::req_to_recourse_not_req_to_pay(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::BillWasNotRequestedToPay))]
-    #[case::req_to_recourse_not_expired_or_rejected(BillValidateActionData { signer_node_id: node_id_test_other(), endorsee_node_id: Some(node_id_test_other()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_other_bill_identified_participant(), valid_bill_identified_participant()))) }, Err(ValidationError::BillIsRequestedToPayAndWaitingForPayment))]
+    #[case::req_to_recourse_not_req_to_pay(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant())) }, Err(ValidationError::BillWasNotRequestedToPay))]
+    #[case::req_to_recourse_not_expired_or_rejected(BillValidateActionData { signer_node_id: node_id_test_and_another(), endorsee_node_id: Some(node_id_test_and_another()), mode: BillValidationActionMode::Deep(BillAction::RequestRecourse(valid_bill_identified_participant(), RecourseReason::Pay(500, CURRENCY_SAT.into()), safe_deadline_ts(RECOURSE_DEADLINE_SECONDS))), ..valid_bill_validate_action_data(add_req_to_pay_block(add_endorse_block(valid_bill_blockchain_issue( valid_bill_issue_block_data(),), valid_and_another_bill_identified_participant(), valid_bill_identified_participant()))) }, Err(ValidationError::BillIsRequestedToPayAndWaitingForPayment))]
     fn test_validate_bill_req_to_recourse_payment_errors(
         #[case] input: BillValidateActionData,
         #[case] expected: Result<(), ValidationError>,

crates/bcr-ebill-core/src/blockchain/bill/chain.rs

@@ -824,6 +824,13 @@ impl BillBlockchain {
     ) -> Result<Vec<PastEndorsee>> {
         let mut result: HashMap<NodeId, PastEndorsee> = HashMap::new();
 
+        let first_version_bill = self.get_first_version_bill(bill_keys)?;
+
+        // if the caller is the drawee (payer), there are no previous endorsees, since they are in the first block
+        if current_identity_node_id == &first_version_bill.drawee.node_id {
+            return Ok(vec![]);
+        }
+
         // we ignore recourse blocks, since we're only interested in previous endorsees before
         // recourse
         let holders = self
@@ -882,7 +889,6 @@ impl BillBlockchain {
             return Ok(vec![]);
         }
 
-        let first_version_bill = self.get_first_version_bill(bill_keys)?;
         // If the drawer is not the drawee, the drawer is the first holder, if the drawer is the
         // payee, they are already in the list
         if first_version_bill.drawer.node_id != first_version_bill.drawee.node_id {
@@ -907,6 +913,9 @@ impl BillBlockchain {
 
         // remove ourselves from the list, if we're somehow on it
         result.remove(current_identity_node_id);
+        // remove the drawee from the list, if they're on it (e.g. if they are payer and contingent holder
+        // but since recourse only happens if the payer already rejected to pay/accept, it doesn't make sense to recourse against them
+        result.remove(&first_version_bill.drawee.node_id);
 
         // sort by signing timestamp descending
         let mut list: Vec<PastEndorsee> = result.into_values().collect();

crates/bcr-ebill-core/src/tests/mod.rs

@@ -130,6 +130,17 @@ pub mod tests {
         }
     }
 
+    pub fn valid_and_another_bill_identified_participant() -> BillIdentParticipant {
+        BillIdentParticipant {
+            t: ContactType::Person,
+            node_id: node_id_test_and_another(),
+            name: Name::new("John Smith").unwrap(),
+            postal_address: valid_address(),
+            email: None,
+            nostr_relays: vec![],
+        }
+    }
+
     pub fn empty_bill_identified_participant() -> BillIdentParticipant {
         BillIdentParticipant {
             t: ContactType::Person,
@@ -199,6 +210,11 @@ pub mod tests {
             .unwrap()
     }
 
+    pub fn node_id_test_and_another() -> NodeId {
+        NodeId::from_str("bitcrt039180c169e5f6d7c579cf1cefa37bffd47a2b389c8125601f4068c87bea795943")
+            .unwrap()
+    }
+
     pub fn node_id_regtest() -> NodeId {
         NodeId::from_str("bitcrr02295fb5f4eeb2f21e01eaf3a2d9a3be10f39db870d28f02146130317973a40ac0")
             .unwrap()