Matthew Zipkin
Matthew Zipkin
concept ACK db38b584e6 Confirmed this branch fixes the issue mentioned in #28250 Code review to follow... ``` { "address": "3JDRACdkEs41yxtYLi6kiqQJdQHLPDxehc", "redeemScript": "602102f293c4d026a536a082453f8327f305ae0a0c59350b396205e15fbec2af67390b2103d7b6a7e972f6e1a532f9f49a7cc35055a0ffd7cec5f81728f6a3fdf4dad9367221021f360d51b8eb43422fe942c837ad16e36bab87c5f7567609de9a38e205fb51ed2103a778eaee9cc18a4e5e02a78f403082d4e6f3a9c0d9edaea20b9ad86089de7fc92102a3c95e41e8272c6842df8c9da492ef5e03bbc4ca17c22ce1f88570e716e187db21032e1d465bc5cdea674337934fb861d097dd7ab20289e4d41de14a6cafcd61d30a210370fd35da88778a805ce9acc88c0020e3b14cacaa30f99d829207e1896d782ea12103878a4b831af4fee069964448ccca999071633fcc751d296e8c5325167274febf210248f8621596029568070dda40bc1e3ace78df320b5ebf0d6a431536e9af3ec8942103f757260835b7e439b244a9b10f5645966d7dcff9d2648c400a0d6827f4a64ac421023b3972554e123985b6c3d8575e6fd3384f59a58ef91e0d39ba5905dad49e73722102e32652928f21764e02822d39ffd1075c428e372a06fe7247023d5b60f1d0159621020e002818c48b1d1766f6196e09db90f51e155bbd49be0ad743d1fa6b7e8efcae2103725d482f0c77f4feb1ecbea28f036b30889f462222002e88aa8f8346dce9447421031fa2798d9f40bc2746f91f300460be9091e10f1b4baa37843cd41c965b0f9b41210235b7b545398ce9692f319e6eefe114eae50f008f46d33baf19418e0f17e3bc652102ff72a2b54370c0b1501228b6a210bc36a16535cc358bf7839c85a6ae2c41725c210282ce44556f5a81c7a698dc8fdba12089740df420af93002e80a752020e5d939e210279f5b244862e27f69284befb0299909139b899d2702b3e74b4317ed5b2947e03210293d0b090906afdee07dabedf34798293246ce06ade2e3887302f289008f25f4d0114ae", "descriptor": "sh(wsh(multi(16,02f293c4d026a536a082453f8327f305ae0a0c59350b396205e15fbec2af67390b,03d7b6a7e972f6e1a532f9f49a7cc35055a0ffd7cec5f81728f6a3fdf4dad93672,021f360d51b8eb43422fe942c837ad16e36bab87c5f7567609de9a38e205fb51ed,03a778eaee9cc18a4e5e02a78f403082d4e6f3a9c0d9edaea20b9ad86089de7fc9,02a3c95e41e8272c6842df8c9da492ef5e03bbc4ca17c22ce1f88570e716e187db,032e1d465bc5cdea674337934fb861d097dd7ab20289e4d41de14a6cafcd61d30a,0370fd35da88778a805ce9acc88c0020e3b14cacaa30f99d829207e1896d782ea1,03878a4b831af4fee069964448ccca999071633fcc751d296e8c5325167274febf,0248f8621596029568070dda40bc1e3ace78df320b5ebf0d6a431536e9af3ec894,03f757260835b7e439b244a9b10f5645966d7dcff9d2648c400a0d6827f4a64ac4,023b3972554e123985b6c3d8575e6fd3384f59a58ef91e0d39ba5905dad49e7372,02e32652928f21764e02822d39ffd1075c428e372a06fe7247023d5b60f1d01596,020e002818c48b1d1766f6196e09db90f51e155bbd49be0ad743d1fa6b7e8efcae,03725d482f0c77f4feb1ecbea28f036b30889f462222002e88aa8f8346dce94474,031fa2798d9f40bc2746f91f300460be9091e10f1b4baa37843cd41c965b0f9b41,0235b7b545398ce9692f319e6eefe114eae50f008f46d33baf19418e0f17e3bc65,02ff72a2b54370c0b1501228b6a210bc36a16535cc358bf7839c85a6ae2c41725c,0282ce44556f5a81c7a698dc8fdba12089740df420af93002e80a752020e5d939e,0279f5b244862e27f69284befb0299909139b899d2702b3e74b4317ed5b2947e03,0293d0b090906afdee07dabedf34798293246ce06ade2e3887302f289008f25f4d)))#xfjxuvgn" } ```
I wrote a test for RPC `signrawtransactionwithkey` that covers legacy P2SH with 15 and 16 public keys. Both calls succeed even though the latter produces a scriptSig that exceeds `MAX_STANDARD_SCRIPTSIG_SIZE`...
> The confusing point about this command is that it returns the post-processed tx hex regardless of what it was able to do internally or if the script passes the...
@stickies-v @mzumsande thanks for your review and feedback, I refactored the PR so we evict a random peer when forced if all other peers are protected. It is MUCH simpler...
Thanks @stickies-v nits addressed! 🙏
@LarryRuane thanks for reviewing. I actually had removed the templating in the [very first version](https://github.com/bitcoin/bitcoin/compare/e71d495ffbda3bc072bbaecd7580809d5087f9e6..c826187b5070ce89edcde0536183714a1c2e3207#diff-2404112ebf57bee5f9a16f1a6e1ecfc27a981d37a1c0ff202b4cd9bdfa3e48ccL77) of this branch but after changing the approach I just left it alone. The function...
@LarryRuane updated PR description to clarify where we get the random peer, added comment about the possible nullopt return value, and removed the default `force` value in `SelectNodeToEvict()`
@vasild @mzumsande Added a commit that restricts the aggressive eviction to new net permission `ForceInbound` which leaves `NoBan` unchanged.
@stickies-v and @LarryRuane thanks for your review. I re-ordered the commits so the history makes more sense now. Also edited the title and description of the PR since we are...
> Would be better to score the peers rather than choose one at random. I had logic for this initially but preferred [this suggestion](https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1431340479) by @mzumsande