## Issue being fixed or feature implemented
fix typos in getrawtransaction and decoderawtransaction help texts
## What was done?
tweak field name to match
https://github.com/dashpay/dash/blob/develop/src/core_write.cpp#L192
## How Has This Been Tested?
## Breaking Changes
n/a
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
should fix "qdata: Already received" discouraging issue
the root of the issue is that we remove expired requests on
UpdatedBlockTip which is too late sometimes.
## What was done?
replacing expired requests with a new one in RequestQuorumData kind of
does the same (drops the expired request) but without waiting for
UpdatedBlockTip
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [ ] I have assigned this pull request to a milestone
8a22fd01140bd957036fc00419b147e4268ae9b1 avoided os-dependant path (Ferdinando M. Ametrano)
Pull request description:
The current code fails on windows because of the forward slashes; using os.path.join solves the problem and it is in general more robust
ACKs for top commit:
MarcoFalke:
ACK 8a22fd01140bd957036fc00419b147e4268ae9b1
Tree-SHA512: 813f27aea33f97c8afac52e716a55fc5d7fb69621023aba99d40df7e1d145e0ec8d1eee49ddd403b219bf0e0e168e0e987b05c78eaef611f744d99bf2fc8bc91
ca185cf5a14b16d61814d7172284bc8efcd28b69 doc: Document differences in bitcoind and bitcoin-qt locale handling (practicalswift)
Pull request description:
Document differences in `bitcoind` and `bitcoin-qt` locale handling.
Since this seems to be the root cause to the locale dependency issues we've seen over the years I thought it was worth documenting :)
Note that 1.) `QLocale` (used by Qt), 2.) C locale (used by locale-sensitive C standard library functions/POSIX functions and some parts of the C++ standard library such as `std::to_string`) and 3.) C++ locale (used by the C++ input/output library) are three separate things. This comment is about the perhaps surprising interference with the C locale (2) that takes place as part of the Qt initialization.
ACKs for top commit:
hebasto:
re-ACK ca185cf5a14b16d61814d7172284bc8efcd28b69
Tree-SHA512: e51c32f3072c506b0029a001d8b108125e1acb4f2b6a48a6be721ddadda9da0ae77a9b39ff33f9d9eebabe2244c1db09e8502e3e7012d7a5d40d98e96da0dc44
## Issue being fixed or feature implemented
RPC help has a wrongly generated RPC help output in CLI and online help:
https://dashcore.readme.io/docs/core-api-ref-remote-procedure-calls-wallet#getwalletinfo
New version:
```
...
"hdaccounts" : [ (json array)
{ (json object)
"hdaccountindex" : n, (numeric) the index of the account
"hdexternalkeyindex" : n, (numeric) current external childkey index
"hdinternalkeyindex" : n (numeric) current internal childkey index
},
...
],
...
```
against old version:
```
...
"" : [ (json array)
{ (json object)
"hdaccountindex" : n, (numeric) the index of the account
"hdexternalkeyindex" : n, (numeric) current external childkey index
"hdinternalkeyindex" : n (numeric) current internal childkey index
},
...
],
...
```
## What was done?
Add a missing name `hdaccounts` for that key for `getwalletinfo` RPC
## How Has This Been Tested?
Run a command `help getwalletinfo` for old and for new versions.
## Breaking Changes
No breaking changes. It doesn't change rpc, only change text description
(help).
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
add a bias to IsExpired to avoid potential timing issues where nodeA thinks it's been 300 seconds but nodeB only thinks it's been 295 for some reason
## What was done?
## How Has This Been Tested?
## Breaking Changes
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
fix `p2p_quorum_data.py` test broken by #5276
## What was done?
adjust data request expiration timeout in tests
## How Has This Been Tested?
`./test/functional/test_runner.py p2p_quorum_data.py`
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
fix `p2p_quorum_data.py` test broken by #5276
## What was done?
adjust data request expiration timeout in tests
## How Has This Been Tested?
`./test/functional/test_runner.py p2p_quorum_data.py`
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Added logs with requested parameters (`llmqType`, `quorumHash`,
`proRegTx`) when sending `qgetdata` for better troubleshooting.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
`GetProjectedMNPayees` wasn't projecting MN payees correctly.
## What was done?
HPMNs are now added 4 times before sorting the return list.
In addition, the case of last payee being HPMN and having still pending
payments is handled.
## How Has This Been Tested?
Tested on Masternodes / Next Payment tab on Qt client on Testnet.
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Added RPC `cleardiscouraged` which clears internally the list of
discouraged peers.
Note: Implementation of a `listdiscouraged` RPC is not possible because
the internal data structure used for discouraged peers is a Bloom
filter.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
- [x] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
Currently, we store internally the nodes that already requested
`QGETDATA` for the same Quorum.
If data for the same Quorum is requested twice from the same `proRegTx`,
then the requester is P2P misbehaved.
## What was done?
Some data like `VerificationVector` and `EncryptedContributions` are not
instantly available.
This PR does not misbehave nodes for requesting data that weren't
available when asked.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
We are seeing lots of "invalid mnauth" on testnet.. We should be logging
this anyhow
## What was done?
Add some logging the the mnauth sig isn't valid
## How Has This Been Tested?
make check
## Breaking Changes
None
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
add a bias to IsExpired to avoid potential timing issues where nodeA thinks it's been 300 seconds but nodeB only thinks it's been 295 for some reason
## What was done?
## How Has This Been Tested?
## Breaking Changes
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
`GetProjectedMNPayees` wasn't projecting MN payees correctly.
## What was done?
HPMNs are now added 4 times before sorting the return list.
In addition, the case of last payee being HPMN and having still pending
payments is handled.
## How Has This Been Tested?
Tested on Masternodes / Next Payment tab on Qt client on Testnet.
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
Currently, we store internally the nodes that already requested
`QGETDATA` for the same Quorum.
If data for the same Quorum is requested twice from the same `proRegTx`,
then the requester is P2P misbehaved.
## What was done?
Some data like `VerificationVector` and `EncryptedContributions` are not
instantly available.
This PR does not misbehave nodes for requesting data that weren't
available when asked.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
RPC help has a wrongly generated RPC help output in CLI and online help:
https://dashcore.readme.io/docs/core-api-ref-remote-procedure-calls-wallet#getwalletinfo
New version:
```
...
"hdaccounts" : [ (json array)
{ (json object)
"hdaccountindex" : n, (numeric) the index of the account
"hdexternalkeyindex" : n, (numeric) current external childkey index
"hdinternalkeyindex" : n (numeric) current internal childkey index
},
...
],
...
```
against old version:
```
...
"" : [ (json array)
{ (json object)
"hdaccountindex" : n, (numeric) the index of the account
"hdexternalkeyindex" : n, (numeric) current external childkey index
"hdinternalkeyindex" : n (numeric) current internal childkey index
},
...
],
...
```
## What was done?
Add a missing name `hdaccounts` for that key for `getwalletinfo` RPC
## How Has This Been Tested?
Run a command `help getwalletinfo` for old and for new versions.
## Breaking Changes
No breaking changes. It doesn't change rpc, only change text description
(help).
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
914923d125f5d17b39b4dc05f666d130e80a68b2 Add setting as known type (Peter Bushnell)
Pull request description:
When loading old wallets I get "Unknown wallet records" showing up in the log file. The key that is adding to the unknown record count is "setting", this is a known key removed in the 0.6 release of Bitcoin in the commit linked below. The "setting" key is not known to the wallet anymore, like "acentry" which is not added as an unknown record, but the "setting" key was used in previous versions of Bitcoin.
972060ce0e (diff-8094838580e1bb7a3bb8fc78dcebc733)
ACKs for top commit:
laanwj:
ACK 914923d125f5d17b39b4dc05f666d130e80a68b2, this code change is straightforward enough and I don't think it makes sense to warn about this key being present.
meshcollider:
ACK 914923d125f5d17b39b4dc05f666d130e80a68b2
Tree-SHA512: 6346690c05cebae2dcd868512322bf5250f6fbd07abb5e747065444185d3f69e19e1a99e3f38d6e34535ffd6979b2297100ba9c7da8e45ca792598eded5ae0d3
bf3be5297a746982cf8e83f45d342121e5665f80 [qa] Ensure we don't generate a too-big block in p2sh sigops test (Suhas Daftuar)
Pull request description:
There's a bug in the loop that is calculating the block size in the p2sh sigops test -- we start with the size of the block when it has no transactions, and then increment by the size of each transaction we add, without regard to the changing size of the encoding for the number of transactions in the block.
This might be fine if the block construction were deterministic, but the first transaction in the block has an ECDSA signature which can be variable length, so we see intermittent failures of this test when the initial transaction has a 70-byte signature and the block ends up being one byte too big.
Fix this by double-checking the block size after construction.
ACKs for top commit:
jonasschnelli:
utACK bf3be5297a746982cf8e83f45d342121e5665f80
jnewbery:
tested ACK bf3be5297a746982cf8e83f45d342121e5665f80
Tree-SHA512: f86385b96f7a6feafa4183727f5f2c9aae8ad70060b574aad13b150f174a17ce9a0040bc51ae7a04bd08f2a5298b983a84b0aed5e86a8440189ebc63b99e64dc
e263a343d4b6a2622df6bb734cd9d51a0d20a663 test: rpc_users: Make variable names more clear. (Carl Dong)
830dc2dd0fccb7f3ec49ff7233a188d92c541e7e test: rpc_users: Also test rpcauth.py with specified password. (Carl Dong)
c73d871799982ca29c29cef90e1a78814cf34019 test: rpc_users: Add function for testing auth params. (Carl Dong)
604e2a997ff26202dd0fa1932d60dc14cc53ac6d test: rpc_users: Add function for auth'd requests. (Carl Dong)
Pull request description:
Fixes#14758
First two commits are tidy-ups which I feel are worthwhile as they are very straightforward, cut down the file by 50%, and made the final diff more minimal. Happy to squash after review.
ACKs for top commit:
laanwj:
ACK e263a343d4b6a2622df6bb734cd9d51a0d20a663
Tree-SHA512: aa75c48570a87060238932d4c68e17234e158077f6195fb4917367e1ecc565e3cd8dd0ae51f9159ddd3d03742739680391bc1246454302db22d4a608c0633e80
e8fabd9253400a7c3fe45b34bc572eb00ff5522d build: prune dbus from depends (fanquake)
Pull request description:
Since #8210 (59d063d076), we've been passing `-dbus-runtime` when configuring Qt.
```
qtbase-opensource-src-5.9.7 $ ./configure -h | grep -i dbus
-no-dbus ............. Do not build the Qt D-Bus module
-dbus-linked ......... Build Qt D-Bus and link to libdbus-1 [auto]
-dbus-runtime ........ Build Qt D-Bus and dynamically load libdbus-1 [no]
```
This means we don't actually seem to be using the `D-Bus` we build in depends. This was pointed out by theuni at the time, [here](https://github.com/bitcoin/bitcoin/pull/7993#issuecomment-223114395) and [here](https://github.com/bitcoin/bitcoin/pull/8210#issuecomment-226930545), but was never followed up. dongcarl also bought it up as part of #16150.
I've tested building and running `bitcoin-qt` using depends on Debian. Needs further testing.
ACKs for top commit:
laanwj:
code review ACK e8fabd9253400a7c3fe45b34bc572eb00ff5522d
Tree-SHA512: 164e6e52b6f97c04aef42bd185e2a157bc1a42103840f9404c5a795749f45a8c2c35f35873395a3a56398b3cd5955496b90d9c885d929b434c9bc871695abe20
fab0c820fa4c0c3227eec85c64310a3bf938a149 rpc: Clarify that block count means height excl genesis (MarcoFalke)
Pull request description:
There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.
However, it really returns the height, which is `0` for the genesis block.
So clarify that and also remove the misleading "longest blockchain" comment.
Finally, fix the wallet test that incorrectly used this rpc.
ACKs for top commit:
instagibbs:
utACK fab0c820fa
promag:
ACK fab0c82, sorry for the misconception.
Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
0f459d868d85053f1cc066ea9099793f88cbd655 fix an undefined behavior in uint::SetHex (Kaz Wesley)
Pull request description:
Decrementing psz beyond the beginning of the string is UB, even though
the out-of-bounds pointer is never dereferenced.
I don't think any clang sanitizer covers this, so I don't see any way a test could catch the original behavior.
ACKs for top commit:
promag:
utACK 0f459d8.
l2a5b1:
utACK 0f459d868d85053f1cc066ea9099793f88cbd655
Tree-SHA512: 388223254ea6e955f643d2ebdf74d15a3d494e9f0597d9f05987ebb708d7a1cc06ce64bd25d447d75b5f5561bdae9630dcf25adb7bd75f7a382298b95d127162
01174596e69568c434198a86f54cb9ea6740e6c2 signrawtransactionwithkey: report error when missing redeemScript/witnessScript param (Anthony Towns)
Pull request description:
Adding support for "witnessScript" as an alternative to "redeemScript" when using "signrawtransactionwithkey" meant that the `RPCTypeCheckObj()` call in `SignTransaction` can't error out just because either parameter is missing -- it's only a problem if both are missing, which isn't a state `RPCTypeCheckObj()` tests for. This results in the regression described in #16249. This patch adds some code to test for this case and give a similar error, namely:
error code: -8
error message:
Missing redeemScript/witnessScript
Fixes: #16249
ACKs for top commit:
meshcollider:
utACK 01174596e6
promag:
ACK 01174596e. Could also write test without `dict`/`del`:
Tree-SHA512: cf51346b7dea551b7f18f2a93c2a336a293b2535c62c03a5263cd2be8c58cf0cc302891da659c167e88ad1a68a756472c3c07e99f71627c61d32886fc5a3a353
303372c41a8d5c58a46cf9ed595e30e67bd0bc99 docs: Improve netaddress comments (Carl Dong)
Pull request description:
Improves comments for `netaddress`, making them available to Doxygen.
I think this is worthwhile because a lot of the code require some context (e.g., A lot of the things that we do to fit hostnames and tor addresses into `CNetAddr` is non-obvious, and documenting it is beneficial).
ACKs for commit 303372:
Tree-SHA512: 2a35784a01ed8ec5fdbe111a540192d31bde16afa96e4be97b0385daf290fc7469a66d7cb8905a70b920fad6a0e7400ca4e5da082d6e4af1d1aaccc0e8297720
## Issue being fixed or feature implemented
Indentation was wrong
## What was done?
## How Has This Been Tested?
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Added RPC `cleardiscouraged` which clears internally the list of
discouraged peers.
Note: Implementation of a `listdiscouraged` RPC is not possible because
the internal data structure used for discouraged peers is a Bloom
filter.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
- [x] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Added logs with requested parameters (`llmqType`, `quorumHash`,
`proRegTx`) when sending `qgetdata` for better troubleshooting.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
We are seeing lots of "invalid mnauth" on testnet.. We should be logging
this anyhow
## What was done?
Add some logging the the mnauth sig isn't valid
## How Has This Been Tested?
make check
## Breaking Changes
None
## Checklist:
- [x] 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
b7ecef1ddf0c9f1f53ab220bee2e19a6b8978e34 guix: ignore additioanl failing certvalidator test (fanquake)
Pull request description:
Backports 8588591965 from #24057 so that from-scratch Guix builds for the Darwin host aren't broken due to a (very recently) expired certificate causing one of the python-certvalidator tests to fail. Kept separate from #23276 because that hasn't gotten review attention, and I don't think we should leave `22.x` Darwin Guix builds broken for any longer than we have to.
Fixes#24110.
```bash
======================================================================
ERROR: test_revocation_mode_soft (tests.test_validate.ValidateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/test_validate.py", line 85, in test_revocation_mode_soft
validate_path(context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 50, in validate_path
return _validate_path(validation_context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 358, in _validate_path
raise PathValidationError(pretty_message(
certvalidator.errors.PathValidationError: The path could not be validated because the end-entity certificate expired 2022-01-14 12:00:00Z
```
Guix Build:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
359755bffecc64b4c005c5cdee3824190f6b1759dbc6c20034476dcc06413959 guix-build-b7ecef1ddf0c/output/dist-archive/bitcoin-b7ecef1ddf0c.tar.gz
0c6700270ec75991d70a97cad77e22cc00553f812edb56c1bac5ef6421f963e1 guix-build-b7ecef1ddf0c/output/x86_64-apple-darwin/SHA256SUMS.part
87d4637a87959a304422550edf87feda3953d7305894154a6a2d413cc0dd2034 guix-build-b7ecef1ddf0c/output/x86_64-apple-darwin/bitcoin-b7ecef1ddf0c-osx-unsigned.dmg
9cabae32689bd5f93e7faaaf341827f1c4069a63ab6f74276564e47819343b6c guix-build-b7ecef1ddf0c/output/x86_64-apple-darwin/bitcoin-b7ecef1ddf0c-osx-unsigned.tar.gz
bb5fb113bc022a305e49783d0ba48be90aca61e4a942beeb45206dbc5b91ca6e guix-build-b7ecef1ddf0c/output/x86_64-apple-darwin/bitcoin-b7ecef1ddf0c-x86_64-apple-darwin.tar.gz
```
ACKs for top commit:
MarcoFalke:
Concept ACK b7ecef1ddf0c9f1f53ab220bee2e19a6b8978e34
Tree-SHA512: 8f761fece405b3b974b9f42ab4ebf8995d3284ce7bfb0556ff8459e1a7d30f8bd2f407cb5651e9fa1094c493148bba7a8918a251b54a83efe12acfaf3d39f350
96cc6bb04f7e173e1f7637b780ac00fc75486671 guix/prelude: Override VERSION with FORCE_VERSION (Carl Dong)
Pull request description:
```
Previously, if the builder exported $VERSION in their environment (as
past Gitian-building docs told them to), but their HEAD does not
actually point to v$VERSION, their build outputs will differ from those
of other builders.
This is because the contrib/guix/guix-* scripts only ever act on the
current git worktree, and does not try to check out $VERSION if $VERSION
is set in the environment.
Setting $VERSION only makes the scripts pretend like the current
worktree is $VERSION.
This problem was seen in jonatack's attestation for all.SHA256SUMS,
where only his bitcoin-22.0rc3-osx-signed.dmg differed from everyone
else's.
Here is my deduced sequence of events:
1. Aug 27th: He guix-builds 22.0rc3 and uploads his attestations up to
guix.sigs
2. Aug 30th, sometime after POSIX time 1630310848: he pulls the latest
changes from master in the same worktree where he guix-built 22.0rc3
and ends up at 7be143a960e2
3. Aug 30th, sometime before POSIX time 1630315907: With his worktree
still on 7be143a960e2, he guix-codesigns. Normally, this would result
in outputs going in guix-build-7be143a960e2, but he had
VERSION=22.0rc3 in his environment, so the guix-* scripts pretended
like he was building 22.0rc3, and used 22.0rc3's guix-build directory
to locate un-codesigned outputs and dump codesigned ones.
However, our SOURCE_DATE_EPOCH defaults to the POSIX time of HEAD
(7be143a960e2), which made all timestamps in the resulting codesigned
DMG 1630310848, 7be143a960e2's POSIX timestamp. This differs from the
POSIX timestamp of 22.0rc3, which is 1630348517. Note that the
windows codesigning procedure does not consider SOURCE_DATE_EPOCH.
We resolve this by only allowing VERSION overrides via the FORCE_VERSION
environment variable.
```
Please ignore the branch name, it's not relevant to the change.
ACKs for top commit:
fanquake:
ACK 96cc6bb04f7e173e1f7637b780ac00fc75486671 - Also makes sense given there are Guix build guides recommending to set `VERSION` as part of the process. i.e https://gist.github.com/hebasto/7293726cbfcd0b58e1cfd5418316cee3.
Tree-SHA512: 9dca3fc637ce11049286a3ebee3cd61cce2125fc51d31cf472fbed7f659e1846fc44062753e0e71bfaec9e7fbab6f040bb88d9d4bc4f8acb28c6890563584acf
