948f1134bc gui: add missing translation.h include to fix build (fanquake)
Pull request description:
After #19176, building the gui on Bionic is failing with:
```bash
CXX qt/qt_libbitcoinqt_a-guiutil.o
qt/bitcoin.cpp: In function 'int GuiMain(int, char**)':
qt/bitcoin.cpp:460:35: error: 'Untranslated' was not declared in this scope
node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
```
The merge commit also failed to compile with the same error:
https://travis-ci.org/github/bitcoin/bitcoin/jobs/696627543
ACKs for top commit:
hebasto:
ACK 948f1134bc, tested on Linux Mint 19.3 (x86_64): it fixes compiling error with the `--disable-wallet` configure option.
Tree-SHA512: db0197b110b3a7d05af2ceb29fbe9eeb6521d28f53b6267aa6d07a975886adb5c6485af79506ab6c66ed101e32292feeaff3707cdbc11432e5b97400953d5631
After #19176, building the gui on Bionic is failing with:
```bash
CXX qt/qt_libbitcoinqt_a-guiutil.o
qt/bitcoin.cpp: In function 'int GuiMain(int, char**)':
qt/bitcoin.cpp:460:35: error: 'Untranslated' was not declared in this scope
node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
```
The merge commit also failed to compile with the same error:
https://travis-ci.org/github/bitcoin/bitcoin/jobs/696627543
6fe989054f refactor: Change Node::initError to take bilingual_str (Wladimir J. van der Laan)
425e7cb8cf refactor: Put`TryParsePermissionFlags` in anonymous namespace (Wladimir J. van der Laan)
77b79fa6ef refactor: Error message bilingual_str consistency (Wladimir J. van der Laan)
Pull request description:
A straightforward and hopefully uncontroversial refactor to improve consistency.
- Move the decision whether to translate an individual error message to where it is defined. This simplifies call sites: no more `InitError(Untranslated(SomeFunction(...)))`.
- Make all functions in `util/error.h` consistently return a `bilingual_str`. We've decided to use this as error message type so let's roll with it.
This has no functional changes: no messages are changed, no new translation messages are defined.
Also make a function static that can be static.
ACKs for top commit:
MarcoFalke:
ACK 6fe989054f🔣
hebasto:
ACK 6fe989054f, tested on Linux Mint 19.3 (x86_64).
Tree-SHA512: 1dd123ef285c4b50bbc429b2f11c9a63aaa669a84955a0a9b8134e9dc141bc38f863f798e8982ac68bbe83170e1067a87d1a87fe7f791928b7914e10bbc2ef8d
6fc641644f change blacklist to blocklist (TrentZ)
Pull request description:
Let's use a more appropriate and clear word and discard the usage of the blacklist. Blocklist is clear and shall make everyone happy.
ACKs for top commit:
amitiuttarwar:
ACK 6fc641644f
jonatack:
ACK 6fc641644f git grep shows these two lines to be the only uses of the word in the codebase other than for specifying colors for the GUI.
sipsorcery:
ACK 6fc641644f due to easy change.
Tree-SHA512: 12fd55ad5c79f1a227da90c7fa730972aae6b74ab1f9df79ec1e7d0eca05c383ef7d6ef5f353620a01da344db915005339b62ca0884179d0f47fbefb084c9efc
- Move the decision whether to translate an error message to where it is
defined. This simplifies call sites: no more `InitError(Untranslated(...))`.
- Make all functions in `util/error.h` consistently return a
`bilingual_str`. We've decided to use this as error message type so
let's roll with it.
This has no functional changes: no messages are changed, no new
translation messages are defined.
bacbfb61ee refactor: Replace RecursiveMutex with Mutex in warnings.cpp (Hennadii Stepanov)
Pull request description:
The functions that could lock this mutex, i.e., `SetMiscWarning()`, `{S,G}etfLargeWorkForkFound()`, `SetfLargeWorkInvalidChainFound()`, `GetWarnings()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_warnings_mutex` could be a non-recursive mutex.
Related to #19180.
ACKs for top commit:
laanwj:
Code review ACK bacbfb61ee
MarcoFalke:
ACK bacbfb61ee , reviewed with -W --word-diff-regex=. 🎿
Tree-SHA512: cc06d3d30e4051115d176dcfbd496c8562a70087369bccde756c1de42d7dc3f415ef20d3d69ad2599c1d0cd4228d604d7564adc17beac7b6ff92b924b8c20d54
b00266fe0c refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner)
Pull request description:
This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module.
For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash:
dcacea096e/src/consensus/tx_verify.cpp (L32)
ACKs for top commit:
Empact:
Code Review ACK b00266fe0c
Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
34645c4dd0 Test txinwitness is accessible on coinbase vin (Rod Vagg)
3e4421070a Expose txinwitness for coinbase in JSON form (Rod Vagg)
Pull request description:
## Rationale
The CLI can provide you with everything about transactions and blocks that you need to reconstruct the block structure and raw block itself **except** for the witness commitment nonce which is stored in the `scriptWitness` of the coinbase and is not printed. You could manually parse the raw `"hex"` fields for transactions if you really wanted to, but this seems to defeat the point of having a JSONification of the raw block/transaction data.
Without the nonce you can't:
1. calculate and validate the witness commitment yourself, you can generate the witness tx merkle root but you don't have the nonce to combine it with
2. reconstruct the raw block form because you don't have `scriptWitness` stack associated with the coinbase (although you know how big it will be and can guess the common case of `[0x000...000]`)
I'm building some archiving tooling for block data and being able to do a validated two-way conversion is very helpful.
## What
This PR simply makes the `txinwitness` field not dependent on whether we are working with the coinbase or not. So you get it for the coinbase as well as the rest.
## Examples
Common case of a `[0x000...000]` nonce: 00000000000000000000140a7289f3aada855dfd23b0bb13bb5502b0ca60cdd7
```json
"vin": [
{
"coinbase": "0368890904c1fe8d5e2f706f6f6c696e2e636f6d2ffabe6d6d5565843a681160cf7b08b1b74ac90a719e6d6ab28c16d336b924f0dc2fcabdc6010000000000000051bf2ad74af345dbe642154b2658931612a70d195e007add0100ffffffff",
"txinwitness": [
"0000000000000000000000000000000000000000000000000000000000000000"
],
"sequence": 4294967295
}
],
...
```
Novel nonce value: 000000000000000000008c31945b2012258366cc600a3e9a3ee0598e8f797731
```json
"vin": [
{
"coinbase": "031862082cfabe6d6d80c099b5e21f4c186d54eb292e17026932e52b1b807fa1380574c5adc1c843450200000000000000",
"txinwitness": [
"5b5032506f6f6c5d5b5032506f6f6c5d5b5032506f6f6c5d5b5032506f6f6c5d"
],
"sequence": 4294967295
}
],
...
```
## Alternatives
This field could be renamed for the coinbase, `"witnessnonce"` perhaps. It could also be omitted when null/zero (`0x000...000`).
## Tests
This didn't break any tests and I couldn't find an obvious way to include a test for this. If this is desired I'd apreicate some pointers.
ACKs for top commit:
MarcoFalke:
ACK 34645c4dd0
Tree-SHA512: b192facc1dfd210a5ec3f0d5d1ac6d0cae81eb35be15eaa71f60009a538dd6a79ab396f218434e7e998563f7f0df2c396cc925cb91619f6841c5a67806148c85
34e641a564 test: Remove unnecessary disconnect_nodes call in rpc_psbt.py (Danny Lee)
e6e7abd51a test: remove redundant two-way disconnect_nodes calls (Danny Lee)
a9bd1f9adf test: warn if nodes not connected before disconnect_nodes (Danny Lee)
Pull request description:
There's no harm in calling `disconnect_nodes` for nodes that weren't connected (in this case it's a no-op). However, detecting this case and logging a warning can help ensure that tests are behaving as expected.
In addition, since `disconnect_nodes` works bidirectionally, I removed all instances of this pattern:
```
disconnect_nodes(self.nodes[0], 1)
disconnect_nodes(self.nodes[1], 0)
```
ACKs for top commit:
MarcoFalke:
review ACK 34e641a564👔
amitiuttarwar:
ACK 34e641a564. Thanks for this test improvement!
Tree-SHA512: 344855ceb46c012d43c13d7c09f44d32dcb7645706d10ae1e4645d9edca54c6c6c13fee26b79480755cdfcdf39b4b5770b36bb03ce71ba002d5be8a27fe008af
cb38b069b0 util: Don't reference errno when pthread fails. (MIZUTA Takeshi)
Pull request description:
Pthread library does not set errno.
Pthread library's errno is returned by return value.
ACKs for top commit:
practicalswift:
ACK cb38b069b0 -- patch looks correct
MarcoFalke:
review ACK cb38b069b0
hebasto:
ACK cb38b069b0, only squashed commits since the [previous](https://github.com/bitcoin/bitcoin/pull/19194#pullrequestreview-425831739) review.
Tree-SHA512: e6c950e30726e5031db97a7b84c8a9215da5ad3e5d233bcc349f812ad15957ddfe378e26d18339b9e0a5dcac2f50b47a687b87a6a6beaf6139df84f31531321e
fa2c2b50d8 doc: Extract net permissions doc (MarcoFalke)
Pull request description:
Moving the documentation of each flag form the already over-large init.cpp into the net permissions module should clean up the code a bit. Moreover, making the documentation available is also required for an (currently imaginary) `setnetpermissions` RPC.
ACKs for top commit:
Sjors:
re-utACK fa2c2b50d8
Tree-SHA512: c0a75facc9768913c28d2ffcdfaad8d60f7604d5584ee546adaf77d270563558d361aeaf354e49e349aca7e2e80814b27ffc24247e7b4f045c63cbdc079b449f
cc5c0d2299 refactor: Fix formatting of timedata.cpp (Hennadii Stepanov)
c2410ceb84 refactor: Replace RecursiveMutex with Mutex in timedata.cpp (Hennadii Stepanov)
Pull request description:
Only `GetTimeOffset()` and `AddTimeData()` functions lock this mutex. They do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_timeoffset_mutex` could be a non-recursive mutex.
Related to #19180.
ACKs for top commit:
MarcoFalke:
ACK cc5c0d2299 , checked the second commit with --word-diff-regex=. --ignore-all-space -U0 🦉
vasild:
ACK cc5c0d22 verified that recursion is not happening
Tree-SHA512: 38f6df689374d4a1a0e9aedb3ed5e885d8285c4da6b75f9bc84ae036936a159ef8276462db33b4f4dd5c71c6312fa9b45380f7a5726959665bc71dc39031be88
78c8f4fe11 refactor: Replace RecursiveMutex with Mutex in netbase.cpp (Hennadii Stepanov)
Pull request description:
The functions that could lock this mutex, i.e., `{S,G}etProxy()`, `{S,G}etNameProxy()`, `HaveNameProxy()`, `IsProxy()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_proxyinfo_mutex` could be a non-recursive mutex.
Related to #19180.
ACKs for top commit:
MarcoFalke:
ACK 78c8f4fe11 , reviewed with the -W git option 👮
vasild:
ACK 78c8f4fe verified that recursion does not happen
Tree-SHA512: fc077fb371f38af5d05f1383c6bebf9926167c257892936fefd2d4fe6f679ca40124d25099e09f645d8ec266df222f96c5d0f9fd39eddcad15cbde0b427bc205
1a9ef1d398 refactor: Replace RecursiveMutex with Mutex in Shutdown() (Hennadii Stepanov)
Pull request description:
Step by step, going to replace all of the `RecursiveMutex` instances with the `Mutex` ones throughout the code base :)
Not sure if it is possible in all cases though...
This one is a low-hanging fruit.
ACKs for top commit:
MarcoFalke:
ACK 1a9ef1d398 Shutdown is not recursive, so the same thread can never lock twice (UB)
vasild:
ACK 1a9ef1d3 verified manually that `Shutdown()` is not called from places that could be called from inside `Shutdown()`.
Tree-SHA512: 362a507b1a6f97dc351f708224aedbfe4bee03c4398f394d78ee31c24d76a7012ffff0e6766866cd5fd9a8e0d8840f05a2741111fe583aa20d45f0af3df0dcfa
Pthread library does not set errno.
Pthread library's errno is returned by return value.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
fac6b9b938 test: Avoid overwriting the NodeContext member of the testing setup (MarcoFalke)
fa16e7816b build: Add -Wshadow-field (MarcoFalke)
Pull request description:
Adding this warning will eliminate unexpected test failures and hard to review code. Moreover, there shouldn't be a use case in Bitcoin Core that relies on fields to be shadowed.
ACKs for top commit:
fanquake:
ACK fac6b9b938 - Warnings compiling fa16e7816b are below. No warnings with fac6b9b938. The `-Wshadow-field` diagnostic has been available in Clang since 5.0.0. It's not available for GCC.
practicalswift:
ACK fac6b9b938 -- patch looks correct
hebasto:
ACK fac6b9b938, tested on Linux Mint 19.3 (x86_64):
Tree-SHA512: 824874ca10877efea7463cf934a2953147f3f99c486f04679426c14ff968975d8652cbba5729bfb7cb2c86c637ade5d1e5b873d611c06bad013a7cad8427e2bf
c514a4f59a doc: release note for `db` log category removal (Jon Atack)
4c0c89307d log: remove deprecated `db` log category (Jon Atack)
Pull request description:
The `db` log category was renamed to `walletdb` (like `coindb`) in #17410 and its upcoming removal announced in the 0.20 release notes.
```
- The `-debug=db` logging category has been renamed to
`-debug=walletdb` to distinguish it from `coindb`. The `-debug=db`
option has been deprecated and will be removed in the next major
release. (#17410)
```
This PR removes the warning and reverts to the usual behavior for an unrecognised log category.
```
$ bitcoin-cli logging '["db"]'
error code: -8
error message:
unknown logging category db
```
```
$ ./src/bitcoind -debug=db
Warning: Unsupported logging category -debug=db.
2020-06-07T15:30:45Z Bitcoin Core version v0.20.99.0-4c0c89307d (debug build)
2020-06-07T15:30:45Z Warning: Unsupported logging category -debug=db.
2020-06-07T15:30:45Z Assuming ancestors of block 0000000000000000000f2adce67e49b0b6bdeb9de8b7c3d7e93b21e7fc1e819d have valid signatures.
2020-06-07T15:30:45Z Setting nMinimumChainWork=00000000000000000000000000000000000000000e1ab5ec9348e9f4b8eb8154
2020-06-07T15:30:45Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2020-06-07T15:30:45Z Using RdSeed as additional entropy source
```
ACKs for top commit:
MarcoFalke:
ACK c514a4f59a🔄
Tree-SHA512: fd62fd7ae0dc65446ba4401d75b4047e055396a33f7f1b176e79a7753250aec2a474ae604163d3f7e68710443c0ed2f45e44435d15f35612d794807e2142d5a3
fa4cd1fdae ci: Switch to bitcoincore.org download (MarcoFalke)
Pull request description:
bitcoin.org is down and not in our control, so it seems odd to rely on it for our ci infrastructure
ACKs for top commit:
troygiorshev:
ACK fa4cd1f
Tree-SHA512: f9f0e9c69a52b8b1906ceae195e8bcc189799fb39be921b26e3a37d1f8f3999831f86c96c3546848c0d01429c36cfb2d7c5f314655ac5282d3e8e4cdd838960e
501e6ab4e7 doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call (Calvin Kim)
Pull request description:
Rationale: When ```bitcoin-cli help verifychain``` is called, the user doesn't get any documentation about the ```checklevel``` argument, leading to issues like #18995.
This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels.
ACKs for top commit:
jonatack:
ACK 501e6ab4e7 `git diff 292ed3c 501e6ab` shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway
MarcoFalke:
ACK 501e6ab4e7🚝
Tree-SHA512: 09239f79c25b5c3022b8eb1f76198ba681305d7e8775038e46becffe5f6a14c572e0c5d06b0723fe9d4a015ec42c9f7ca7b80a2a93df0b1b66f5a84a80eeeeb1
fa9604c46f doc: noban precludes maxuploadtarget disconnects (MarcoFalke)
fa3999fe35 net: Reformat excessively long if condition into multiple lines (MarcoFalke)
Pull request description:
Whitelisting has been replaced by permission flags, so properly document this. See also #10131
ACKs for top commit:
hebasto:
ACK fa9604c46f, I have reviewed the code and it looks OK, I agree it can be merged.
ariard:
ACK fa9604c
Tree-SHA512: 5aee917ab9817719f01ec155487542118e17fa3d145ae7e4bc0e872b2cec39cde9e7fbdee2ae77e9a52700dd8bcc366de4224152e08e709d44d08e0d2f19c613
5d77549d8b doc: Add mypy to test dependencies (Hennadii Stepanov)
7dda912e1c test: Do not swallow flake8 exit code (Hennadii Stepanov)
Pull request description:
After #18210 the `flake8` exit code in `test/lint/lint-python.sh` just not used that makes the linter broken.
This PR:
- combines exit codes of `flake8` and `mypy` into the `test/lint/lint-python.sh` exit code
- documents `mypy` as the test dependency
ACKs for top commit:
MarcoFalke:
Approach ACK 5d77549d8b, fine with me
practicalswift:
ACK 5d77549d8b
Tree-SHA512: e948ba04dc4d73393967ebf3c6a26c40d428d33766382a0310fc64746cb7972e027bd62e7ea76898b742a656cf7d0fcda2fdd61560a21bfd7be249cea27f3d41
4a7253ab6c Remove g_rpc_chain global (Russell Yanofsky)
e783197bf0 refactor: replace RegisterWalletRPCCommands with GetWalletRPCCommands (Russell Yanofsky)
Pull request description:
Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference.
This PR is a followup to #18740 removing the g_rpc_node global.
Some later PRs will follow this up and move more wallet globals to the WalletContext struct.
ACKs for top commit:
MarcoFalke:
ACK 4a7253ab6c🎋
ariard:
Code Review ACK 4a7253a, feel free to ignore comment it's super nit.
Tree-SHA512: 5bb5561c89f81811ca5232a58bf450e230d4218e62471c03227d142395fd36131672e99cb88329b33b9680a235db01e8b9d1c1e2a18288349e57205528deabab
0012471391 build: turn on --enable-c++17 by --enable-fuzz (Vasil Dimov)
Pull request description:
Fuzzing code uses C++17 specific code (e.g. std::optional), so it is not
possible to compile with --enable-fuzz and without --enable-c++17.
Thus, turn on --enable-c++17 whenever --enable-fuzz is used.
ACKs for top commit:
hebasto:
ACK 0012471391, tested on Linux Mint 19.3 (x86_64); verified that it fails to compile with `--enable-fuzz` and without `--enable-c++17` on master.
Tree-SHA512: 290531ea8d79de3b9251ea4ad21e793478b18150cc0124eea1e50c3a4ed92bab89c3e70ed0aa526906f8723ea952cdba4268f1560ae4be9bd25b9e4f9b97436c
Fuzzing code uses C++17 specific code (e.g. std::optional), so it is not
possible to compile with --enable-fuzz and without --enable-c++17.
Thus, turn on --enable-c++17 whenever --enable-fuzz is used.
fa7e002d52 ci: tsan with wallet (MarcoFalke)
Pull request description:
ACKs for top commit:
practicalswift:
ACK fa7e002d52 -- patch looks correct and Travis is happy
hebasto:
ACK fa7e002d52, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 1138459bbef72f402f32dae1e28d96f174901d4248d959b538b973747c8b06f221ecd81a386a1f915c0a2faaefb9fb8f11e3be39e6c5e2468bf9ae43d9f97757
f46b678acf qt: lock cs_main, m_cached_tip_mutex in that order (Vasil Dimov)
Pull request description:
Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in
the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up
in a deadlock.
`ClientModel::m_cached_tip_blocks` is protected by
`ClientModel::m_cached_tip_mutex`. There are two access paths that
lock the two mutexes in opposite order:
```
validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main
validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip()
ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost
...
qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex
```
and
```
qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex
qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash()
interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main
```
From `debug.log`:
```
POTENTIAL DEADLOCK DETECTED
Previous lock order was:
m_cs_chainstate validation.cpp:2851
(1) cs_main validation.cpp:2868
::mempool.cs validation.cpp:2868
(2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255
Current lock order is:
(2) m_cached_tip_mutex qt/clientmodel.cpp:119
(1) ::cs_main interfaces/node.cpp:200
```
The possible deadlock was introduced in #17993
ACKs for top commit:
jonasschnelli:
Tested ACK f46b678acf
Tree-SHA512: 904f24b39bdc97c4d0ecb897a6980d8d479814535eb167e23105238800ea2f1f85273e3370cf894db58bc597f94c4f2e81fb68d0ff3362d468c16af5ce8f5d78
c4b574899a gui: Add Close All Wallets action (João Barbosa)
f30960adc0 gui: Add closeAllWallets to WalletController (João Barbosa)
Pull request description:
This PR adds the action to close all wallets.
<img width="405" alt="Screenshot 2020-06-01 at 01 06 12" src="https://user-images.githubusercontent.com/3534524/83365986-25a8b980-a3a4-11ea-9613-24dcd8eaa55c.png">
ACKs for top commit:
jonasschnelli:
Tested ACK c4b574899a
Tree-SHA512: 049ad77ac79949fb55f6bde47b583fbf946f4bfaf3d56d768e85f813d814cff0fe326b700f7b5e383cda4af7b5666e13043a6aaeee3798a69fc94385d88ce809
89f9fef1f7 refactor: Specify boost/thread/thread.hpp explicitly (Hennadii Stepanov)
fad8c890f5 txdb: Remove unused boost/thread (MarcoFalke)
faa958bc28 txindex: Remove unused boost/thread (MarcoFalke)
Pull request description:
There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points
However, non-boost threads such as `std::thread` or the `main()` thread can obviously not be interrupted. So remove all unused boost/thread from methods that are never executed in a `boost::thread`.
Most of them were accompanied by a `ShutdownRequested` anyway. So even if the current thread was a `boost::thread`, the interruption point would be redundant. (We only interrupt threads during shutdown)
ACKs for top commit:
fanquake:
ACK 89f9fef1f7
hebasto:
ACK 89f9fef1f7, tested on Linux Mint 19.3 (x86_64), verified shutdown in different scenarios.
Tree-SHA512: 17221dadedf2d107e5bda9e4f371cc4f8ffce6ad27cae41aa2b8f1150d8f1adf23d396585ca4a2dd25b1dc6f0d5c81fecd950d8557966ccb45a6d4a85a331d90