fix(core/transactions): resolve or remove TODOs (#5500)

Description
---

remove TODO that no longer makes sense
WalletOutput does not contain secret keys, remove TODO
resolve TODO in
single_recipient_with_rewindable_change_and_receiver_outputs_bulletproofs
remove TODO, looks like TransactionOutput::verify_mask is still useful

Motivation and Context
---
Remove TODOs from core/transactions.

These remain:
- unblinded_output: TODO: Try to get rid of 'Serialize' and
'Deserialize' traits here; see related comment at 'struct
RawTransactionInfo' (wallet ffi requires serialization to JSON)
- validator signature: TODO: pass in commitment instead of arbitrary
message
- verify_validator_node_signature: 
```rust
    // TODO(SECURITY): Signing this with a blank msg allows the signature to be replayed. Using the commitment
            //                 is ideal as uniqueness is enforced. However, because the VN and wallet have different
            //                 keys this becomes difficult. Fix this once we have decided on a solution.
```

How Has This Been Tested?
---

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

base_layer/core/src/transactions/transaction_components/kernel_features.rs

@@ -25,7 +25,6 @@ use serde::{Deserialize, Serialize};
 
 bitflags! {
     /// Options for a kernel's structure or use.
-    /// TODO:  expand to accommodate Tari DAN transaction types, such as namespace and validator node registrations
     #[derive(Deserialize, Serialize, BorshSerialize, BorshDeserialize)]
     pub struct KernelFeatures: u8 {
         /// Coinbase transaction

base_layer/core/src/transactions/transaction_components/side_chain/validator_node_signature.rs

@@ -45,7 +45,6 @@ impl ValidatorNodeSignature {
         let (secret_nonce, public_nonce) = PublicKey::random_keypair(&mut OsRng);
         let public_key = PublicKey::from_secret_key(private_key);
         let challenge = Self::construct_challenge(&public_key, &public_nonce, msg);
-        // TODO: Changing to use the new signing API requires a lot of changes
         let signature = Signature::sign_raw(private_key, secret_nonce, &*challenge)
             .expect("Sign cannot fail with 32-byte challenge and a RistrettoPublicKey");
         Self { public_key, signature }

base_layer/core/src/transactions/transaction_components/transaction_output.rs

@@ -336,7 +336,6 @@ impl TransactionOutput {
     }
 
     /// Attempt to verify a recovered mask (blinding factor) for a proof against the commitment.
-    /// TODO: Remove this method when core key manager is fully implemented
     pub fn verify_mask(
         &self,
         prover: &RangeProofService,

base_layer/core/src/transactions/transaction_components/wallet_output.rs

@@ -55,8 +55,6 @@ use crate::{
 
 /// A wallet output is one where the value and spending key (blinding factor) are known. This can be used to
 /// build both inputs and outputs (every input comes from an output)
-// TODO: Try to get rid of 'Serialize' and 'Deserialize' traits here; see related comment at 'struct RawTransactionInfo'
-// #LOGGED
 #[derive(Clone, Serialize, Deserialize)]
 pub struct WalletOutput {
     pub version: TransactionOutputVersion,

base_layer/core/src/transactions/transaction_protocol/sender.rs

@@ -1508,7 +1508,8 @@ mod test {
             .with_lock_height(0)
             .with_fee_per_gram(MicroTari(20))
             .with_change_data(
-                script!(Nop),
+                // "colour" this output so that we can find it later
+                script!(PushInt(1) Drop Nop),
                 inputs!(change_params.script_key_pk),
                 change_params.script_key_id.clone(),
                 change_params.spend_key_id.clone(),
@@ -1584,17 +1585,9 @@ mod test {
         let tx = alice.get_transaction().unwrap();
         assert_eq!(tx.body.outputs().len(), 2);
 
-        // If the first output isn't alice's then the second must be
-        // TODO: Fix this logic when 'encrypted_data.todo_decrypt()' is fixed only one of these will be possible
-        let output_0 = &tx.body.outputs()[0];
-        let output_1 = &tx.body.outputs()[1];
+        let output = tx.body.outputs().iter().find(|o| o.script.size() > 1).unwrap();
 
-        if let Ok((key, _value)) = key_manager_alice.try_output_key_recovery(output_0, None).await {
-            assert_eq!(key, change_params.spend_key_id);
-        } else if let Ok((key, _value)) = key_manager_alice.try_output_key_recovery(output_1, None).await {
-            assert_eq!(key, change_params.spend_key_id);
-        } else {
-            panic!("Could not recover Alice's output");
-        }
+        let (key, _value) = key_manager_alice.try_output_key_recovery(output, None).await.unwrap();
+        assert_eq!(key, change_params.spend_key_id);
     }
 }