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#11394, #15054, #15102, #15893, #16286, #16973, #17111, #17146, #17829, #19072, #19258, #20567 #5720

Merged
merged 12 commits into from
Dec 6, 2023
Merged

backport: bitcoin#11394, #15054, #15102, #15893, #16286, #16973, #17111, #17146, #17829, #19072, #19258, #20567 #5720

merged 12 commits into from
Dec 6, 2023

Conversation

knst
Copy link
Collaborator

@knst knst commented Nov 20, 2023

Issue being fixed or feature implemented

Various tiny backports (most of them trivial) from v18-v21 bitcoin

What was done?

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 Nov 20, 2023
@knst knst requested review from UdjinM6, PastaPastaPasta and kwvg and removed request for kwvg November 20, 2023 15:23
@knst knst marked this pull request as draft November 21, 2023 13:36
@knst knst force-pushed the missing-old-backports branch from 2bae36f to 6098b32 Compare November 24, 2023 17:12
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please rebase.

@knst knst force-pushed the missing-old-backports branch from 6098b32 to f27f772 Compare November 24, 2023 17:28
@knst knst marked this pull request as ready for review November 24, 2023 17:29
UdjinM6
UdjinM6 reviewed Nov 29, 2023
View reviewed changes
test/functional/combine_logs.py
glob = pathlib.Path(tmp_dir).glob('node0/**/debug.log')
path = next(glob, None)
if path:
assert next(glob, None) is None or '/feature_config_args_' in tmp_dir # more than one debug.log, should never happen
Copy link

Choose a reason for hiding this comment

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

16973: where is or '/feature_config_args_' in tmp_dir part coming from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have log collecting by CI even if it's succeed, that's an unique feature of dash.
Bitcoin's implementation fails too for functional test "feature_config_args", but it's never an issue for them. So, that's an workaround for that particular functional test because "never should happen" happens in this particular test but Bitcoin doesn't know it.

(As tested from same revision of Bitcoin collect log scrip lt fails for Bitcoin for this test too just no one noticed it)

UdjinM6
UdjinM6 previously approved these changes Nov 29, 2023
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

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 1, 2023

This pull request has conflicts, please rebase.

@knst knst force-pushed the missing-old-backports branch 2 times, most recently from e0876ac to cbbebf2 Compare December 1, 2023 15:27
UdjinM6
UdjinM6 approved these changes Dec 2, 2023
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.

re-utACK

PastaPastaPasta
PastaPastaPasta approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

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

MarcoFalke and others added 8 commits December 6, 2023 11:40
@PastaPastaPasta
Merge bitcoin#15054: Update copyright headers to 2018
2dc520b 
1a49a0e Bump manpages (DrahtBot)
06ba779 Update copyright headers to 2018 (DrahtBot)

Pull request description:

  * `./contrib/devtools/copyright_header.py update ./`
  * `./contrib/devtools/gen-manpages.sh`

Tree-SHA512: ca0dc5e97f4c33814d4ccd17769bbf2d23a99a71d62534fe1064fedfe47de3b5c30caf9b6deb0d70bf125e08c7ae6335ac4fcded918049d6b63b13b319d798e3
@PastaPastaPasta
Merge bitcoin#15102: test: Run invalid_txs.InputMissing test in featu…
549a358 
…re_block

fac4e73 test: Run invalid_txs.InputMissing test in feature_block (MarcoFalke)

Pull request description:

Tree-SHA512: 24c3f519ba0cf417b66e0df6f5ddc0430e3f419af4705a9c85096da47ff4d8f51487d65b68f3f993800003b3f936d95d8a0bade846e1b45f95b2bdbecc9ebab7
@PastaPastaPasta
partial Merge bitcoin#15893: Add test for superfluous witness record …
9d4282c 
…in deserialization

cc556e4 Add test for superfluous witness record in deserialization (Gregory Sanders)
25b0786 Fix missing input template by making minimal tx (Gregory Sanders)

Pull request description:

  Adds coverage for changed behavior in bitcoin#14039

ACKs for commit cc556e:
  MarcoFalke:
    utACK cc556e4

Tree-SHA512: 3404c8f75e87503983fac5ae27d877309eb3b902f2ec993762911c71610ca449bef0ed98bd17e029414828025b2713e1bd012e63b2a06497e34f1056acaa6321
@fanquake @PastaPastaPasta
Merge bitcoin#16286: refactoring: wallet: Fix GCC 7.4.0 warning
ab36852 
d8bd97d Fix GCC 7.4.0 warning (Hennadii Stepanov)

Pull request description:

  Having bitcoin#13756 and bitcoin#16239 merged cause GCC warning:
  ```
  $ gcc --version
  gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
  ...
  $ make -j 4 > /dev/null
  wallet/wallet.cpp: In member function ‘CWallet::Balance CWallet::GetBalance(int, bool) const’:
  wallet/wallet.cpp:2269:45: warning: enumeral and non-enumeral type in conditional expression [-Wextra]
       isminefilter reuse_filter = avoid_reuse ? 0 : ISMINE_USED;
                                   ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
  ```

  Fixed with this PR.

ACKs for top commit:
  practicalswift:
    utACK d8bd97d
  promag:
    ACK d8bd97d.
  kallewoof:
    utACK d8bd97d
  meshcollider:
    Trivial utACK bitcoin@d8bd97d

Tree-SHA512: 2fb315ac82f290c8b9f4e48d1b4526b9babe0717c68593c7bc55cd6c289e64b6322aba72984f39291a9254b57d3f6ba8dbfe03799f510c0c1dc108b21b286732
@PastaPastaPasta
Merge bitcoin#16973: test: Fix combine_logs.py for AppVeyor build
921c93d 
d478a47 test: Fix combine_logs.py for AppVeyor build (Martin Zumsande)

Pull request description:

  Fixes bitcoin#16894

  This fixes the problem of AppVeyor builds not showing `debug.log` if a functional test fails, because the windows separator `\` doesn't work together with the regex in `combine_logs.py`.

  A fix was already attempted in  bitcoin#16896, however, that PR became inactive and was marked "up for grabs", plus it's a really small change.

  As suggested by jamesob, this PR uses `pathlib`: For the glob and to convert the path to a posix-style string, it leaves the regex as is (in contrast to bitcoin#16896 which adjusted the regex).

  I tested this locally on Windows and Ubuntu.

Top commit has no ACKs.

Tree-SHA512: 603b4359b6009b6da874c30f69759acda03730ee5747898a0fe957a5fc37ee9ba07858c6aa2169bf4c40521f37e47138e8314d698652ea2760fa0a3f76b890bd
@laanwj @PastaPastaPasta
Merge bitcoin#17111: doc: update bips.md with buried BIP9 deployments
c2c865e 
fa6ed82 doc: update bips.md with buried BIP9 deployments (MarcoFalke)

Pull request description:

  Also, remove the activation heights, as they can be retrieved from `./src/chainparams.cpp` (if needed)

ACKs for top commit:
  laanwj:
    ACK fa6ed82, needs backport to 0.19 I guess.

Tree-SHA512: 9c069cc14589a3e2309d76f042677c024a9e14d16dbfccef54c4a2963ca7853d01f042b0237e346538c557591b7553deed9dd811ba64bbd0ced88883d562c59a
@fanquake @PastaPastaPasta
Merge bitcoin#17146: github: Add warning for bug reports
9b79d92 
5f40d27 github: Add warning for bug reports (Wladimir J. van der Laan)

Pull request description:

  I've noticed the "Bug" label being added redundantly fairly frequently. I think this might be due to github's templates.

  All in all, the link in https://github.com/bitcoin/bitcoin/issues/new/choose to open a regular issue is a bit hidden from sight. Direct people's attention to it.

ACKs for top commit:
  practicalswift:
    ACK 5f40d27 - currently it is very easy to miss the tiny "Open a regular issue" link :)
  jonasschnelli:
    ACK 5f40d27
  hebasto:
    ACK 5f40d27

Tree-SHA512: e6c94c02f9f7d00621b580d406d03f8754173150bf456409ccc474b76fb93ff857ff4a0c652bf5c03d4f1b97ecf29ae0ff7bf8b763207f9c8522b8dcecc20109
@fanquake @PastaPastaPasta
Merge bitcoin#17829: scripted-diff: Bump copyright of files changed i…
2da9982 
…n 2019

aaaaad6 scripted-diff: Bump copyright of files changed in 2019 (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK aaaaad6
  promag:
    ACK aaaaad6 🎉
  fanquake:
    ACK aaaaad6 - going to merge this now because the year is over and conflicts are minimal.

Tree-SHA512: 58cb1f53bc4c1395b2766f36fabc7e2332e213780a802762fff0afd59468dad0c3265f553714d761c7a2c44ff90f7dc250f04458f4b2eb8eef8b94f8c9891321
fanquake and others added 4 commits December 6, 2023 11:40
@fanquake @PastaPastaPasta
Merge bitcoin#19072: doc: Expand section on Getting Started
0eba155 
facef3d doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2 doc: Expand section on Getting Started (MarcoFalke)
100000d doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e doc: Fix unrelated typos reported by codespell (MarcoFalke)

Pull request description:

  Some random doc changes:

  * Add sections to docs, so that they can be linked to
  * Explain that anyone (even maintainers) are allowed to work on good first issues
  * Expand section on Getting Started slightly

ACKs for top commit:
  hebasto:
    ACK facef3d
  fanquake:
    ACK facef3d

Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
@PastaPastaPasta
Merge bitcoin#11394: Perform a weaker subtree check in Travis
17b67ba 
487aff4 Check subtree consistency in Travis (Pieter Wuille)
e1d0cc2 Improve git-subtree-check.sh (Pieter Wuille)

Pull request description:

  Apparently many of our subtrees get modified by PRs in this repository, without getting noticed.

  To improve upon this:
  * Make git-subtree-check.sh capable of doing a weaker consistency check (that doesn't need access to external repositories), but which should be sufficient to detect unintended changes. It can be fooled by a fake subtree merge commit, but that would hopefully be obvious to reviewers.
  * Make Travis invoke this subtree check for each of our subtrees.

  Note that Travis is currently expected to fail on this PR, as 2 out of 4 subtrees (`src/secp156k1` and `src/univalue` have been modified directly in master).

Tree-SHA512: 465b680392d3daf38a8c1dda77d6f74b1d1c23324c378774777fb95aa673e119a8f7e3ccc124e41d97b5ac8975f3d79f3015797d2d309666582394364917ec4e
@laanwj @PastaPastaPasta
Merge bitcoin#19258: doc: improve subtree check instructions
f706a56 
a4a3fc4 doc: improve subtree check instructions (Sjors Provoost)

Pull request description:

  Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.

  This PR also improves the error message if the subtree commit can't be found.

ACKs for top commit:
  laanwj:
    ACK a4a3fc4
  fanquake:
    ACK a4a3fc4 - this looks ok.

Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
@laanwj @PastaPastaPasta
Merge bitcoin#20567: test: Add option to git-subtree-check to do full…
9fd9e4c 
… check, add help

34c80d9 test: Add option to git-subtree-check to do full check, add help (Wladimir J. van der Laan)

Pull request description:

  This adds a brief help text to `git-subtree-check.sh` and adds an option to do a full remote check instead of having two different code paths with a successful exit status. Also make it explicit that the CI is not doing this.

ACKs for top commit:
  fjahr:
    tested ACK 34c80d9

Tree-SHA512: 20f672fd3b3c1d633eccf9998fdd738194cdd7d10cc206691f2dcc28bbbf8187b8d06b87814f875a06145b179f5ca1f4f4f9922972be72759cf5ac6e0c11abd1
@PastaPastaPasta PastaPastaPasta merged commit 8a18471 into dashpay:develop Dec 6, 2023
5 checks passed
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 UdjinM6 approved these changes

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

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