wallet: Use single FastRandomContext when creating a wallet tx #24560

1. 
   MarcoFalke commented at 2:21 pm on March 14, 2022: member

   Passing around a single randomness context shouldn’t come with any downsides, but documents better where randomness is used and allows the unit test to be deterministic, if they wish to be so.
2. wallet: Pass FastRandomContext& to DiscourageFeeSniping 77773b061c
3. wallet: Pass FastRandomContext& to coin selection fa7deaa046
4. MarcoFalke added the label Refactoring on Mar 14, 2022
5. MarcoFalke renamed this:
   wallet: Pass FastRandomContext& to DiscourageFeeSniping
   wallet: Use single FastRandomContext when creating a wallet tx
   on Mar 14, 2022
6. MarcoFalke added the label Wallet on Mar 14, 2022
7. 
   DrahtBot commented at 2:29 pm on March 14, 2022: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #24494 (wallet: generate random change target for each tx for better privacy by glozow)
   • #24213 (refactor: use Span in random.*; make GetRand a template, remove GetRandInt by PastaPastaPasta)
   • #24128 (wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs by MarcoFalke)
   • #23475 (wallet: add config to prioritize a solution that doesn’t create change in coin selection by brunoerg)
   • #23076 (Pass CFeeRate and CTxMemPool in-params by reference to const by jonatack)
   • #20640 (wallet, refactor: return out-params of CreateTransaction() as optional struct by theStack)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

8. 
   glozow commented at 4:14 pm on March 14, 2022: member

   Concept ACK
9. in src/wallet/coinselection.cpp:194 in fa7deaa046

   190@@ -191,16 +191,14 @@ std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& ut
   191     return std::nullopt;
   192 }
   193 
   194-static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue,
   195+static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue,
   

   ---

   ---

   

   promag commented at 6:28 pm on March 17, 2022:

   nit, any reason to be the 1st arg?

   ---

   

   MarcoFalke commented at 6:33 pm on March 17, 2022:

   It can’t be the last arg, because that requires a default value, so I picked the “least-last” arg (aka first)
10. 
    promag commented at 6:28 pm on March 17, 2022: member

    Code review ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada.
11. 
    glozow commented at 11:56 am on March 21, 2022: member

    light code review ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada
12. 
    achow101 commented at 5:40 pm on March 23, 2022: member

    ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada
13. achow101 merged this on Mar 23, 2022
14. achow101 closed this on Mar 23, 2022

    ---

15. sidhujag referenced this in commit aa27161a51 on Mar 24, 2022
16. MarcoFalke deleted the branch on Mar 24, 2022
17. DrahtBot locked this on Mar 24, 2023

Contributors
MarcoFalke DrahtBot glozow promag achow101

Labels
Refactoring Wallet