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: trivial 2024 07 25 #6158

Merged
merged 11 commits into from
Aug 13, 2024
Merged

backport: trivial 2024 07 25 #6158

merged 11 commits into from
Aug 13, 2024

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Trivial backports; need to see ci pass

What was done?

How Has This Been Tested?

built

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • 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 (for repository code-owners and collaborators only)

@PastaPastaPasta PastaPastaPasta added this to the 21.1 milestone Jul 25, 2024
knst
knst requested changes Jul 26, 2024
View reviewed changes
test/functional/test_framework/test_shell.py Outdated
@@ -16,6 +16,9 @@ class TestShell:
start a single TestShell at a time."""

class __TestShell(BitcoinTestFramework):
def add_options(self, parser):
Copy link
Collaborator

Choose a reason for hiding this comment

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

26689: too early to backport, to do after bitcoin#26480

src/rpc/net.cpp Outdated
@@ -186,6 +186,14 @@ static RPCHelpMan getpeerinfo()
UniValue obj(UniValue::VOBJ);
CNodeStateStats statestats;
bool fStateStats = peerman.GetNodeStateStats(stats.nodeid, statestats);
// GetNodeStateStats() requires the existence of a CNodeState and a Peer object
Copy link
Collaborator

Choose a reason for hiding this comment

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

An earlier version of this PR also made the affected CNodeStateStats fields non-optional (see mzumsande@5f900e2). Since this conflicts with bitcoin#25923 and should be a separate discussion, I removed that commit from this PR.

should include 26515 and bitcoin#26727 in one PR

contrib/devtools/test-security-check.py
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']),
(1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']),
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']),
Copy link
Collaborator

Choose a reason for hiding this comment

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

note 26835 - it tested with CI and succeed -- see ci/test/00_setup_env_mac_host.sh: export RUN_SECURITY_TESTS="true"

Should we still do guix build for PR?

@UdjinM6 UdjinM6 modified the milestones: 21.1, 21.2 Aug 8, 2024
MacroFake and others added 11 commits August 12, 2024 11:52
@PastaPastaPasta
Merge bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and inval…
9590929 
…id input test coverage

2ef5294 rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack)
734b966 test: add getblockfrompeer coverage of invalid inputs (Jon Atack)

Pull request description:

  The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs.

  Fix all issues.

ACKs for top commit:
  brunoerg:
    ACK 2ef5294

Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25
@PastaPastaPasta
Merge bitcoin#26759: test: Drop no longer needed race:epoll_ctl TSa…
df2f533 
…n suppression

a3f5e54 test: Drop no longer needed `race:epoll_ctl` TSan suppression (Hennadii Stepanov)

Pull request description:

  The removed suppression seems no needed.

  I cannot point the exact commit/PR which makes this change possible.

Top commit has no ACKs.

Tree-SHA512: 8ee79cbdb2bc62796d72c69be4a818379132eae47be33951e8b9d224b049ff77e867004801c7cb0cc564a5374f318dafd9142b5231e9bd428f80acc75253933e
@fanquake @PastaPastaPasta
Merge bitcoin#26772: contrib: fix sha256 check in install_db4.sh for …
7f2b934 
…FreeBSD

22e9afe use sha256 command instead of sha256sum on FreeBSD (Murray Nesbitt)

Pull request description:

  The FreeBSD version of `sha256sum` takes different arguments than the GNU version.

  The `sha256_check` function in `contrib/install_db4.sh` has code specific to FreeBSD, however it doesn't get reached because while the `sha256sum` command does exist on FreeBSD, it is incompatible and results in an error:

  ```
  sha256sum: option requires an argument -- c
  usage: sha256sum [-pqrtx] [-c file] [-s string] [files ...]
  ```

  This change moves the FreeBSD-specific code before the check for the `sha256sum` command.

  Fixes: bitcoin#26774

Top commit has no ACKs.

Tree-SHA512: 2485e2e7d8fdca3b072b29fb22bbdfd69e520740537b331b33c64cc645b63da712cfa63a23bdf039bbc92a6558fc7bf03323a51784bf601ff360ff0ef59506c8
@glozow @PastaPastaPasta
Merge bitcoin#26603: doc: CalculateSequenceLocks: prevHeights entries…
092ddc3 
… are set to 0, not removed

f537127 doc: fix: prevHeights entries are set to 0, not removed (stickies-v)

Pull request description:

  In [`CalculateSequenceLocks`](https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.h#L69) no items are removed from `prevHeights`, they are just set to 0:

  https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.cpp#L69-L73

  This PR updates the docs to reflect the actual implementation. Seems to have been wrongly documented since introduction in bitcoin#7184 already ([implementation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R742-R749) and [documentation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R712-R713))

ACKs for top commit:
  hebasto:
    ACK f537127

Tree-SHA512: 3661501660f6832b2116fd83466ffe95a60b341c14cb09a37489e2a587bea3290b0528690120a0f644c3eea02177aa1fb8968258482fa43b0303e016abb17418
@achow101 @PastaPastaPasta
Merge bitcoin#26809: compat: use STDIN_FILENO over 0
864d02e 
585c672 compat: use STDIN_FILENO over 0 (fanquake)

Pull request description:

  This is already used throughout this file, and is self-documenting.

ACKs for top commit:
  john-moffett:
    ACK 585c672
  achow101:
    ACK 585c672
  hebasto:
    ACK 585c672, I have reviewed the code and it looks OK, I agree it can be merged.
  kristapsk:
    utACK 585c672
  aureleoules:
    ACK 585c672

Tree-SHA512: c0114ae896ba5404be70b804ee9f454d213f1d789c8f5a578c422dd15a308a214e6851fee76c0ec736a212bc86fb33ec17af1b22e5d23422c375ca4458251356
@PastaPastaPasta
Merge bitcoin#26818: test: Fix feature_startupnotify intermittent issue
d1b93c7 
fac810b test: Fix feature_startupnotify intermittent issue (MarcoFalke)

Pull request description:

  Might fix bitcoin#25644

ACKs for top commit:
  aureleoules:
    ACK fac810b
  brunoerg:
    ACK fac810b

Tree-SHA512: 870bf65da8120b6897d02e3bb70eea018d4761396abe64c3533bbc5237e65be9f77d35f62cd5d08cf7132dd53b504bf58229c33e18833c191495ad229c84d7c2
@PastaPastaPasta
Merge bitcoin#26864: doc: net: fix link to onion address encoding sch…
6fe46fc 
…eme [ONIONADDRESS]

3076f18 doc: net: fix link to onion address encoding scheme [ONIONADDRESS] (Sebastian Falbesoner)

Pull request description:

  Instead of referring to a fixed line number to a file in master (which is obviously always quickly outdated), use a permalink tied to the latest commit.

ACKs for top commit:
  vasild:
    ACK 3076f18

Tree-SHA512: 7070a7e47d683b1539f33daa4c67093a87d6121a84430a3b24afee139a7f2b3cab32fcdf0bc561f8e177b5ba864a98e4491e08dac90cdd4bd2e4e6b8fa7e4b14
@PastaPastaPasta
Merge bitcoin#26730: test: add coverage for purpose arg in `listlab…
07f4c39 
…els`

c467cff test: add coverage for `purpose` arg in `listlabels` (brunoerg)

Pull request description:

  This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive).

  https://github.com/bitcoin/bitcoin/blob/dcdfd72861c09a7945b9facc3726177a2d06fa64/src/wallet/rpc/addresses.cpp#L698-L704

ACKs for top commit:
  kristapsk:
    ACK c467cff

Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8
@achow101 @PastaPastaPasta
Merge bitcoin#26821: refactor: Make ThreadHTTP return void
93c4652 
45553e1 refactor: Make `ThreadHTTP` return void (Hennadii Stepanov)

Pull request description:

  The `bool` return value was introduced in 755aa05 (bitcoin#8421).

  It has been not used since 8d3f46e (bitcoin#14670).

  No behavior change.

ACKs for top commit:
  achow101:
    ACK 45553e1
  brunoerg:
    crACK 45553e1
  w0xlt:
    ACK bitcoin@45553e1
  stickies-v:
    ACK 45553e1

Tree-SHA512: 1593a5740e729967fbe1363235cd5b77ecf431b29bc740a89a6c70fc838ad97a2e4a2cd7cd63aa482f7c50bc2ffabc8cd53e8f64d6032603cb3b662229bc3dc2
@PastaPastaPasta
Merge bitcoin#26827: doc: use "std lib clock" over "C++11 clock"
b6bde73 
672f7ad doc: remove usages of C++11 (fanquake)

Pull request description:

  These were new in C++11, and now they are just our standard library.

ACKs for top commit:
  jarolrod:
    re-ACK 672f7ad
  hebasto:
    re-ACK 672f7ad

Tree-SHA512: 7e3b8b0346ba29b19e6d8536700ca510e2b543cdeecd9e740bba71ea6d0133dd96cdaeaa00f371f8ef85913ff5aaabe12878255f393dac7d354a8b89b58d050a
@fanquake @PastaPastaPasta
Merge bitcoin#26835: contrib: add PE Canary check to security-check
0dd997c 
6ba17d4 scripts: add PE Canary check to security-check (fanquake)

Pull request description:

  We should be checking this, same as ELF & MACHO.

  Guix Build:
  ```bash
  6334c001b276ca5f0278092be68bf6d49d9b755bcac893bbd4aa58df57356e40  guix-build-6ba17d4955b7/output/dist-archive/bitcoin-6ba17d4955b7.tar.gz
  e27ad7fffb377bc6264477933859ab47c7283a68fbf86124d3801bc4c8b790dd  guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/SHA256SUMS.part
  ef7b61bd854f0d3c39f356ef85ac18d37c5740874111f5ce46f7ce3381e714ca  guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-debug.zip
  c419324597487f248143a076d6eb2a56b0dbf5ce690ca89afaaee5c6b352e1a1  guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-setup-unsigned.exe
  a18ff1e3026cd9fc08dd7b500c06a343462aef4a37538608d940d1845bcdb94a  guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-unsigned.tar.gz
  7e4ee0669940f4b8c1a12dab836898511a60f06a62057ac03beaca8bb693bfb4  guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64.zip
  ```

ACKs for top commit:
  sipsorcery:
    ACK 6ba17d4.

Tree-SHA512: 1acc24c0cb36dbc30311f4eee64e3d4737c828b97039be0f72cfe061bcb8c4d5c830d7792f503e711e219a62d85b7e07cdff3510cbd4f8d46895a7cb66b88219
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-07-25 branch from 94d4a0c to 0dd997c Compare August 12, 2024 04:52
@PastaPastaPasta PastaPastaPasta requested a review from knst August 12, 2024 06:56
knst
knst approved these changes Aug 13, 2024
View reviewed changes
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK 0dd997c

UdjinM6
UdjinM6 approved these changes Aug 13, 2024
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 0dd997c

@PastaPastaPasta PastaPastaPasta merged commit 6ef28ca into dashpay:develop Aug 13, 2024
8 of 9 checks passed
@PastaPastaPasta PastaPastaPasta deleted the develop-trivial-2024-07-25 branch August 13, 2024 14:06
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@knst knst knst approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@PastaPastaPasta @knst @UdjinM6 @fanquake @glozow @achow101