Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

backport: bitcoin#8456, #15704, #18850, #19241, #19317, #19493, #19739, #19903, #20003 #5813

Merged
merged 9 commits into from
Jan 11, 2024
Merged

backport: bitcoin#8456, #15704, #18850, #19241, #19317, #19493, #19739, #19903, #20003 #5813

merged 9 commits into from
Jan 11, 2024

Conversation

knst
Copy link
Collaborator

@knst knst commented Jan 10, 2024 •
edited
Loading

Issue being fixed or feature implemented

Just regular bitcoin backports, mostly from v21

What was done?

Backport bitcoin#8456 includes only specific helpers that are used in other backports.

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst added this to the 20.1 milestone Jan 10, 2024
@knst knst changed the base branch from master to develop January 10, 2024 19:31
@knst knst marked this pull request as draft January 10, 2024 20:03
@knst knst changed the title backport: bitcoin#8456, #15704, #18850, #19241, #19317, #19493, #19739, #19903, #19971, #20003 backport: bitcoin#8456, #15704, #18850, #19241, #19317, #19493, #19739, #19903, #20003 Jan 10, 2024
@knst knst marked this pull request as ready for review January 10, 2024 21:21
PastaPastaPasta
PastaPastaPasta approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK for merging via merge commit; all of these are pretty trivial; total diff is tiny. CI is happy going to merge now, @UdjinM6 please post-review

laanwj and others added 9 commits January 10, 2024 19:22
@laanwj @PastaPastaPasta
Merge bitcoin#19317: Add a left-justified width field to log2_work co…
bba596d 
…mponent for a uniform debug.log output

c858302 Change format of log2_work for uniform output (zero-padded) (jmorgan)

Pull request description:

  Motivation:
  It's jarring to watch the output of `tail -f ~/btcdata/debug.log` scroll by and very frequently see columns not lining up correctly because `log2_work` somtimes has less precision than 8 digits.

  Current display:
  ```
  2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo)
  2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo)
  ```

  Display with this commit:
  ```
  2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo)
  2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868  tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo)
  ```

ACKs for top commit:
  practicalswift:
    ACK c858302 -- patch looks great :)
  achow101:
    ACK c858302
  laanwj:
    Tested ACK c858302

Tree-SHA512: 16cbe419c4993ad51019c676e8ca409ef1025b803cc598437c780dd7ca003d7e4ad421f451e9a374e0070ee9b3ee601b7aba849e1f346798f9321d1bce5c4401
@meshcollider @PastaPastaPasta
Merge bitcoin#18850: wallet: Fix ZapSelectTx to sync wallet spends
b64081b 
9c59f9c Fix ZapSelectTx to sync wallet spends (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <[email protected]>

ACKs for top commit:
  achow101:
    ACK 9c59f9c
  ryanofsky:
    Code review ACK 9c59f9c. Only change since last review tweaking the for loop as suggested
  jonatack:
    ACK 9c59f9c tested rebased on current master b33136b and the new unit test does indeed fail without the change.
  meshcollider:
    utACK 9c59f9c

Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
@mrbandrews @PastaPastaPasta
partial Merge bitcoin#8456: [RPC] Simplified bumpfee command.
21e1a4a 
It includes only this commit for sake of backporting HasWalletSpent

[RPC] bumpfee

This command allows a user to increase the fee on a wallet transaction T, creating a "bumper" transaction B.
T must signal that it is BIP-125 replaceable.
T's change output is decremented to pay the additional fee.  (B will not add inputs to T.)
T cannot have any descendant transactions.
Once B bumps T, neither T nor B's outputs can be spent until either T or (more likely) B is mined.

Includes code by @jonasschnelli and @ryanofsky
@PastaPastaPasta
Merge bitcoin#19493: wallet: Fix clang build in Mac
c95bbed 
1e58bcc wallet: Fix clang build in Mac (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <[email protected]>

Top commit has no ACKs.

Tree-SHA512: 19312929af14dab97c37cf4547fbd6589a6de960f1a499c2118bb684240639af4b127cf8dc4d201b41d253cfbb645614a0606d4ecce29f300b10c210d38a961b
@fanquake @PastaPastaPasta
Merge bitcoin#15704: Move Win32 defines to configure.ac to ensure the…
feb6363 
…y are globally defined

1ccb9f3 Move Win32 defines to configure.ac to ensure they are globally defined (Luke Dashjr)

Pull request description:

  bitcoin#9245 no longer needs this, since the main `_WIN32_WINNT` got bumped by something else.

  So rather than just lose it, might as well get it merged in independently.

  I'm not aware of any practical effects, but it seems safer to use the same API versions everywhere.

ACKs for top commit:
  fanquake:
    ACK 1ccb9f3 - checked that the binaries produced are the same.

Tree-SHA512: 273e9186579197be01b443b6968e26b9a8031d356fabc5b73aa967fcdb837df195b7ce0fc4e4529c85d9b86da6f2d7ff1bf56a3ff0cbbcd8cee8a9c2bf70a244
@laanwj @PastaPastaPasta
Merge bitcoin#19739: refactor: remove c-string interfaces for DecodeB…
fb274b1 
…ase58{Check}

d3e8adf util: remove c-string interfaces for DecodeBase58{Check} (Sebastian Falbesoner)

Pull request description:

  This micro-PR gets rid of base58 function interfaces that are redundant in terms of c-string / std::string variants; the c-string interface for `DecodeBase58Check` is completely unused outside the base58 module, while the c-string interface for `DecodeBase58` is only used in unit tests, where an implicit conversion to std::string is not problematic.

ACKs for top commit:
  practicalswift:
    ACK d3e8adf -- patch looks correct
  laanwj:
    Code review ACK d3e8adf

Tree-SHA512: 006a4a1e23b11385f60820c188b8e6b1634a182ca36e29a6580f72150214c65a3fdb273ec439165f26ba88a42d2bf5bab1cf3666a9eaee222fb4e1c00aeba433
@fanquake @PastaPastaPasta
Merge bitcoin#19903: Update build-openbsd.md with GUI support
2870a68 
d110200 Add OpenBSD instructions for building the Qt GUI (grubles)

Pull request description:

  Using OpenBSD as a desktop OS is prevalent enough IMO to warrant updating the documentation for building the GUI.

ACKs for top commit:
  fanquake:
    ACK d110200 - looks fine. Have not tested.

Tree-SHA512: a8078334fdd35438bcf87c3f5eae851c2a1ce961eb48ae50770bf2c556489da86b6ee198fe9fb732dcaddb2e0f2f4f55a3126971aae8f7d4e2e320dbb024e204
@laanwj @PastaPastaPasta
Merge bitcoin#19241: help: Generate checkpoint height from chainparams
a5cb057 
916d359 help: Generate checkpoint height from chainparams (Luke Dashjr)

Pull request description:

  Not sure if this is worth putting in Core, but might as well until checkpoints are removed entirely.

ACKs for top commit:
  laanwj:
    re-ACK 916d359

Tree-SHA512: d8eb26b570ee730fdd75ca916507134db5f2f68987a911e33544b7f1c9ccfd1c76b9c9db63056971956b6daf16910f17ecfc197481c2f7b0773afdfbf7d381cf
@laanwj @PastaPastaPasta
Merge bitcoin#20003: net: Exit with error message if -proxy is specif…
4293cbd 
…ied without arguments (instead of continuing without proxy server)

9b4fa0a net: Print error message if -proxy is specified without arguments (instead of continuing without proxy server) (practicalswift)

Pull request description:

  Exit with error message if `-proxy` is specified without arguments (instead of continuing without proxy server).

  Continuing without a proxy server when the end-user has specified `-proxy` may result in accidental loss of privacy. (The end-user might think he/she is using a proxy when he/she is not.)

  Before this patch:

  ```
  $ src/bitcoind -proxy
  …
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -listen=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -upnp=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -discover=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0
  …
  2020-09-23T00:24:33Z init message: Starting network threads...
  ```

  `bitcoind` is now running *without* a proxy server (`GetProxy(…, …) == false`, `HaveNameProxy() == false`, etc.).

  Note that the "-proxy set" log messages above which the end-user might interpret as "good, my traffic is now routed via the proxy".

  After this patch:

  ```
  $ src/bitcoind -proxy
  Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>.
  $ echo $?
  1
  ```

ACKs for top commit:
  laanwj:
    re-ACK 9b4fa0a
  kristapsk:
    ACK 9b4fa0a, I have tested the code.
  hebasto:
    re-ACK 9b4fa0a

Tree-SHA512: 4ba7a011991699a54b5bb87ec68367c681231bf5dcd36f8c89ff9ddc2e8d29df453817b7e362597e652ad6b341a22b7274be0fd78d435e5f0fd8058e5221c4ce
@PastaPastaPasta PastaPastaPasta merged commit a78c28d into dashpay:develop Jan 11, 2024
4 of 5 checks passed
@UdjinM6
Copy link

UdjinM6 commented Jan 11, 2024

post-merge utACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

@UdjinM6 UdjinM6 Awaiting requested review from UdjinM6

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
20.1
Development

Successfully merging this pull request may close these issues.
7 participants
@knst @UdjinM6 @PastaPastaPasta @laanwj @meshcollider @mrbandrews @fanquake