66667130416b86208e01a0eb5541a15ea805ac26 refactor: Rename fs::path::u8string() to fs::path::utf8string() (MarcoFalke)
856c88776f8486446602476a1c9e133ac0cff510 ArgsManager: return path by value from GetBlocksDirPath() (Vasil Dimov)
fa3d9304e80c214c8b073f12a7f4b08c5a94af04 refactor: Remove pre-C++20 fs code (MarcoFalke)
fa00098e1a493aa3cce20335d18e7f5f2fb7a4a8 Add tests for C++20 std::u8string (MarcoFalke)
fa2bac08c22182e738a8cabf1b24a9dbf3b092d2 refactor: Avoid copy/move in fs.h (MarcoFalke)
faea30227ba633da5ab257d0247853e0927244bb refactor: Use C++20 std::chrono::days (MarcoFalke)
Pull request description:
This:
* Removes dead code.
* Avoids unused copies in some places.
* Adds copies in other places for safety.
ACKs for top commit:
achow101:
ACK 66667130416b86208e01a0eb5541a15ea805ac26
ryanofsky:
Code review ACK 66667130416b86208e01a0eb5541a15ea805ac26. Just documentation change since last review.
stickies-v:
re-ACK 66667130416b86208e01a0eb5541a15ea805ac26
Tree-SHA512: 6176e44f30b310d51632ec2d3827c3819905d0ddc6a4b57acfcb6cfa1f9735176da75ee8ed4a4abd1296cb0b83bee9374cc6f91ffac87c19b63c435eeadf3f46
6db04be102807ee0120981a9b8de62a55439dabb Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly (Ryan Ofsky)
213542b625a6a4885fcbdfe236629a5f381eeb05 refactor: Add InitContext function to initialize NodeContext with global pointers (Ryan Ofsky)
feeb7b816affa790e02e7ba0780c4ef33d2310ff refactor: Remove calls to StartShutdown from KernelNotifications (Ryan Ofsky)
6824eecaf1e74624cf149ed20abd9145c49d614a refactor: Remove call to StartShutdown from stop RPC (Ryan Ofsky)
1d92d89edbb1812dc353084c62772ebb1024d632 util: Get rid of uncaught exceptions thrown by SignalInterrupt class (Ryan Ofsky)
ba93966368d3aaa426b97837ef475ec5aa612f5f refactor: Remove call to ShutdownRequested from IndexWaitSynced (Ryan Ofsky)
42e5829d9710ebebda5de356fab01dd7c149d5fa refactor: Remove call to ShutdownRequested from HTTPRequest (Ryan Ofsky)
73133c36aa9cc09546eabac18d0ea35274dd5d72 refactor: Add NodeContext::shutdown member (Ryan Ofsky)
f4a8bd6e2f03e786a84dd7763d1c04665e6371f2 refactor: Remove call to StartShutdown from qt (Ryan Ofsky)
f0c73c1336bee74fe2d58474ac36bca28c219e85 refactor: Remove call to ShutdownRequested from rpc/mining (Ryan Ofsky)
263b23f0082c60516acced1b03abb8e4d8f9ee46 refactor: Remove call to ShutdownRequested from chainstate init (Ryan Ofsky)
Pull request description:
This change drops `shutdown.h` and `shutdown.cpp` files, replacing them with a `NodeContext::shutdown` member which is used to trigger shutdowns directly. This gets rid of an unnecessary layer of indirection, and allows getting rid of the `kernel::g_context` global.
Additionally, this PR tries to improve error handling of `SignalInterrupt` code by marking relevant methods `[[nodiscard]]` to avoid the possibility of uncaught exceptions mentioned https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707.
Behavior is changing In a few cases which are noted in individual commit messages. Particularly: GUI code more consistently interrupts RPCs when it is shutting down, shutdown state no longer persists between unit tests, the stop RPC now returns an RPC error if requesting shutdown fails instead of aborting, and other failed shutdown calls now log errors instead of aborting.
This PR is a net reduction in lines of code, but in some cases the explicit error handling and lack of global shutdown functions do make it more verbose. The verbosity can be seen as good thing if it discourages more code from directly triggering shutdowns, and instead encourages code to return errors or send notifications that could be translated into shutdowns. Probably a number of existing shutdown calls could just be replaced by better error handling.
ACKs for top commit:
achow101:
ACK 6db04be102807ee0120981a9b8de62a55439dabb
TheCharlatan:
Re-ACK 6db04be102807ee0120981a9b8de62a55439dabb
maflcko:
ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗
stickies-v:
re-ACK 6db04be102807ee0120981a9b8de62a55439dabb
Tree-SHA512: 7a34cb69085f37e813c43bdaded1a0cbf6c53bd95fdde96f0cb45346127fc934604c43bccd3328231ca2f1faf712a7418d047ceabd22ef2dca3c32ebb659e634
fa3da629a1aebcb4500803d7417feed8e34285b0 Remove DirIsWritable, GetUniquePath (MarcoFalke)
fad3a9793b71df5bb0b17cc3758cf3466d08c015 Return LockResult::ErrorWrite in LockDirectory (MarcoFalke)
fa0afe740843c308f6287b923f1f4d758cf2a3f6 refactor: Return enum in LockDirectory (MarcoFalke)
Pull request description:
`GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`.
Fix the redundancy by removing everything, except `LockDirectory`.
ACKs for top commit:
TheCharlatan:
Re-ACK fa3da629a1aebcb4500803d7417feed8e34285b0
hebasto:
ACK fa3da629a1aebcb4500803d7417feed8e34285b0, I have reviewed the code and it looks OK.
Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9
Treating std::string as UTF-8 is deprecated in std::filesystem::path
since C++20.
However, it makes this codebase easier to read and maintain to retain
the ability for std::string to hold UTF-8.
Replace exceptions thrown by signal and wait methods with [[nodiscard]] return
values.
This is mostly a refactoring, but there is a slight change of behavior if
AbortShutdown function fails. The original behavior which was unintentionally
changed in #27861 is restored, so it now triggers an assert failure again
instead of throwing an exception. (The AbortShutdown function is only ever
called in the the GUI version of Bitcoin Core when corruption is detected on
loading and the user tries to reindex.)
Problems with using exceptions were pointed out by MarcoFalke in
https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707.
bbbbdb0cd57d75a06357d2811363d30a498f4499 ci: Add filesystem lint check (MarcoFalke)
fada2f91108a56cc5c447bd6b6fac411e4d5cdca refactor: Replace <filesystem> with <util/fs.h> (MarcoFalke)
Pull request description:
Using `std::filesystem` is problematic:
* There is a `fs` namespace wrapper for it. So having two ways to achieve the same is confusing.
* Not using the `fs` wrapper is dangerous and buggy, because it disables known bugs by deleting problematic functions.
Fix all issues by removing use of it and adding a linter to avoid using it again in the future.
ACKs for top commit:
TheCharlatan:
ACK bbbbdb0cd57d75a06357d2811363d30a498f4499
fanquake:
ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 🦀
Tree-SHA512: 0e2d49742b08eb2635e6fce41485277cb9c40fe20b81017c391d3472a43787db1278a236825714ca1e41c9d2f59913865cfb0c649e3c8ab1fb598c849f80c660
af0fca530e4d8311bcb24a14c416e5ad7c30ff78 netbase: use reliable send() during SOCKS5 handshake (Vasil Dimov)
1b19d1117ca5373a15313227b547ef4392022dbd sock: change Sock::SendComplete() to take Span (Vasil Dimov)
Pull request description:
The `Socks5()` function which does the SOCKS5 handshake with the SOCKS5 proxy sends bytes to the socket without retrying partial writes.
`send(2)` may write only part of the provided data and return. In this case the caller is responsible for retrying the operation with the remaining data. Change `Socks5()` to do that. There is already a method `Sock::SendComplete()` which does exactly that, so use it in `Socks5()`.
A minor complication for this PR is that `Sock::SendComplete()` takes `std::string` argument whereas `Socks5()` has `std::vector<uint8_t>`. Thus the necessity for the first commit. It is possible to do also in other ways - convert the data in `Socks5()` to `std::string` or have just one `Sock::SendComplete()` that takes `void*` and change the callers to pass `str.data(), str.size()` or `vec.data(), vec.size()`.
This came up while testing https://github.com/bitcoin/bitcoin/pull/27375.
ACKs for top commit:
achow101:
ACK af0fca530e4d8311bcb24a14c416e5ad7c30ff78
jonatack:
ACK af0fca530e4d8311bcb24a14c416e5ad7c30ff78
pinheadmz:
ACK af0fca530e4d8311bcb24a14c416e5ad7c30ff78
Tree-SHA512: 1d4a53d0628f7607378038ac56dc3b8624ce9322b034c9547a0c3ce052eafb4b18213f258aa3b57bcb4d990a5e0548a37ec70af2bd55f6e8e6399936f1ce047a
bb91131d545d986aab81c4bb13676c4520169259 doc: remove out-of-date external link in src/util/strencodings.h (Jon Atack)
7d494a48ddf4248ef3b1753b6e7f2eeab3a8ecb7 refactor: use string_view to pass string literals to Parse{Hash,Hex} (Jon Atack)
Pull request description:
as `string_view` is optimized to be trivially copiable, whereas the current code creates a `std::string` copy at each call.
These utility methods are called by quite a few RPCs and tests, as well as by each other.
```
$ git grep "ParseHashV\|ParseHashO\|ParseHexV\|ParseHexO" | wc -l
61
```
Also remove an out-of-date external link.
ACKs for top commit:
jonatack:
Rebased per `git range-diff c9273f6 b94581a bb91131` for an include header from the merge of https://github.com/bitcoin/bitcoin/pull/28230. Should be trivial to re-ACK.
maflcko:
lgtm ACK bb91131d545d986aab81c4bb13676c4520169259
ns-xvrn:
ACK bb91131d54
achow101:
ACK bb91131d545d986aab81c4bb13676c4520169259
brunoerg:
crACK bb91131d545d986aab81c4bb13676c4520169259
Tree-SHA512: 9734fe022c9e43fd93c23a917770d332dbbd3132c80a234059714c32faa6469391e59349954749fc86c4ef0b18d5fd99bf8f4b7b82d9f799943799c1253272ae
faa769db5a4c16fd171e9a39c33e245db4e7c134 Fix bugprone-lambda-function-name errors (MarcoFalke)
Pull request description:
Inside a lambda, `__func__` will evaluate to something like `"operator()"`. Fix this by either removing it, or by using the real name.
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html
ACKs for top commit:
TheCharlatan:
ACK faa769db5a4c16fd171e9a39c33e245db4e7c134
darosior:
utACK faa769db5a4c16fd171e9a39c33e245db4e7c134
Tree-SHA512: 0b562bd4ebd7f46ca3ebabeee67851ad30bd522fa57e5010e833b163664e51f5df645ff9ca35d22c3479fb27d9267d4e5d0d417d42729bf3ccf80d7944970e4e
940a49978c70453e1aaf2c4a0bcb382872b844a5 Use type-safe txid types in orphanage (dergoegge)
ed70e6501648466b9ca91a39b83775363e9a726d Introduce types for txids & wtxids (dergoegge)
cdb14d79e809bf7d1612b21b554a9fcfb2ab1c91 [net processing] Use HasWitness over comparing (w)txids (dergoegge)
Pull request description:
We currently have two different identifiers for transactions: `txid` (refering to the hash of a transaction without witness data) and `wtxid` (referring to the hash of a transaction including witness data). Both are typed as `uint256` which could lead to type-safety bugs in which one transaction identifier type is passed where the other would be expected.
This PR introduces explicit `Txid` and `Wtxid` types that (if used) would cause compilation errors for such type confusion bugs.
(Only the orphanage is converted to use these types in this PR)
ACKs for top commit:
achow101:
ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5
stickies-v:
ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5
hebasto:
ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5, I have reviewed the code and it looks OK.
instagibbs:
re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5
BrandonOdiwuor:
re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5
glozow:
reACK 940a49978c
Tree-SHA512: 55298d1c2bb82b7a6995e96e554571c22eaf4a89fb2a4d7a236d70e0f625e8cca62ff2490e1c179c47bd93153fe6527b56870198f026f5ee7753d64d7a424c92
The `ENABLE_TRACING` macro is expected to be defined in the
`config/bitcoin-config.h` header.
Therefore, the current code is error-prone as it depends on whether the
`config/bitcoin-config.h` header was included before or not.
7df450836969b81e98322c9a09c08b35d1095a25 test: improve sock_tests/move_assignment (Vasil Dimov)
5086a99b84367a45706af7197da1016dd966e6d9 net: remove Sock default constructor, it's not necessary (Vasil Dimov)
7829272f7826511241defd34954e6040ea963f07 net: remove now unnecessary Sock::Get() (Vasil Dimov)
944b21b70ae490a5a746bcc1810a5074d74e9d34 net: don't check if the socket is valid in ConnectSocketDirectly() (Vasil Dimov)
aeac68d036e3cff57ce155f1a904d77f98b357d4 net: don't check if the socket is valid in GetBindAddress() (Vasil Dimov)
5ac1a51ee5a57da59f1ff1986b7d9054484d3c80 i2p: avoid using Sock::Get() for checking for a valid socket (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
Peeking at the underlying socket file descriptor of `Sock` and checkig if it is `INVALID_SOCKET` is bad encapsulation and stands in the way of testing/mocking/fuzzing.
Instead use an empty `unique_ptr` to denote that there is no valid socket where appropriate or outright remove such checks where they are not necessary.
The default constructor `Sock::Sock()` is unnecessary now after recent changes, thus remove it.
ACKs for top commit:
ajtowns:
ACK 7df450836969b81e98322c9a09c08b35d1095a25
jonatack:
ACK 7df450836969b81e98322c9a09c08b35d1095a25
Tree-SHA512: 9742aeeeabe8690530bf74caa6ba296787028c52f4a3342afd193b05dbbb1f6645935c33ba0a5230199a09af01c666bd3c7fb16b48692a0d185356ea59a8ddbf
This allows us to reference assumeutxo configuration by blockhash as
well as height; this is helpful in future changes when we want to
reference assumeutxo configurations before the block index is loaded.
ad0c469d98c51931b98b7fd937c6ac3eeaed024e wallet: Use CTxDestination in CRecipient rather than scriptPubKey (Andrew Chow)
07d3bdf4ebc06825ea24ab6f7c87aef6a22238c6 Add PubKeyDestination for P2PK scripts (Andrew Chow)
1a98a51c666e9ae77364115775ec2e0ba984e8e0 Allow CNoDestination to represent a raw script (Andrew Chow)
8dd067088d41f021b357d7db5fa5f0a9f61edddc Make WitnessUnknown members private (Andrew Chow)
Pull request description:
For silent payments, we want to provide a `SilentPaymentsDestination` to be used as the recipient, which requires `CRecipient` to use something other than just the `scriptPubKey` as we cannot know the output script for a silent payment prior to transaction creation. `CTxDestination` seems like the obvious place to add a `SilentPaymentsDestination` as it is our internal representation of an address.
In order to still allow paying to arbitrary scriptPubKeys (e.g. for data carrier outputs, or the user hand crafted a raw transaction that they have given to `fundrawtransaction`), `CNoDestination` is changed to contain raw scripts.
Additionally, P2PK scripts are now interpreted as a new `PubKeyDestination` rather than `PKHash`. This results in some things that would have given an address for P2PK scripts to no longer do so. This is arguably more correct.
`ExtractDestination`'s behavior is slightly changed for the above. It now returns `true` for those destinations that have addresses, so P2PK scripts now result in `false`. Even though it returns false for `CNoDestination`, the script will now be included in that `CNoDestination`.
Builds on #28244
ACKs for top commit:
josibake:
ACK ad0c469d98
Tree-SHA512: ef3f8f3c7284779d9806c77c85b21caf910a79a1f7e7f1b51abcc0d7e074f14e00abf30f625a13075e41d94dad6202c10ddff462c0ee74c2ca4aab585b145a52
3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Do not use std::vector = {} to release memory (Pieter Wuille)
Pull request description:
It appears that invoking `v = {};` for an `std::vector<...> v` is equivalent to `v.clear()`, which does not release its allocated memory. There are a number of places in the codebase where it appears to be used for that purpose however (mostly written by me). Replace those with `std::vector<...>{}.swap(v);` (using a helper function `ClearShrink` in util/vector.h).
To explain what is going on: `v = {...};` is equivalent in general to `v.operator=({...});`. For many types, the `{}` is converted to the type of `v`, and then assigned to `v` - which for `std::vector` would ordinarily have the effect of clearing its memory (constructing a new empty vector, and then move-assigning it to `v`). However, since `std::vector<T>` has an `operator=(std::initializer_list<T>)` defined, it has precedence (since no implicit conversion is needed), and with an empty list, that is equivalent to `clear()`.
I did consider using `v = std::vector<T>{};` as replacement for `v = {};` instances where memory releasing is desired, but it appears that it does not actually work universally either. `V{}.swap(v);` does.
ACKs for top commit:
ajtowns:
utACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e
stickies-v:
ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e
theStack:
Code-review ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e
Tree-SHA512: 6148558126ec3c8cfd6daee167ec1c67b360cf1dff2cbc132bd71768337cf9bc4dda3e5a9cf7da4f7457d2123288eeba77dd78f3a17fa2cfd9c6758262950cc5
All code in this repo uses <util/fs.h>, except for a few lines. This is
confusing and potentially dangerous, if the safe <util/fs.h> wrappers
are not used.
This commit makes compat.h no longer a required include for users of the
libbitcoinkernel. Including compat.h imports a bunch of
platform-specific definitions.
This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.
Make sure that nothing else can change WitnessUnknown's data members by
making them private. Also change the program to use a vector rather than
C-style array.
`Sock::Get()` was used only in `sock.{cpp,h}`. Remove it and access
`Sock::m_socket` directly.
Unit tests that used `Get()` to test for equality still verify that the
behavior is correct by using the added `operator==()`.
5197660e947435e510ef3ef72be8be8dee3ffa41 tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings (Martin Leitner-Ankerl)
Pull request description:
Fixes#26916 by disabling the warning `-Wgnu-zero-variadic-macro-arguments` when clang is used as the compiler.
Also see the comments
* Proposed changes in the bug https://github.com/bitcoin/bitcoin/issues/26916#issuecomment-1480997053
* Proposed changes when moving to a variadic maro: https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1155488768
ACKs for top commit:
hebasto:
ACK 5197660e947435e510ef3ef72be8be8dee3ffa41, I've reconsidered my [comment](https://github.com/bitcoin/bitcoin/pull/27401#issuecomment-1507142439) and I think the current localized approach is optimal.
fanquake:
ACK 5197660e947435e510ef3ef72be8be8dee3ffa41 - checked that this fixes the warnings under Clang.
Tree-SHA512: c3dda3bcbb2540af6283ffff65885a9937bfdaaef3b00dc7d60b9f9740031d5c36ac9cb3d3d8756dbadce4812201a9754f5b8770df0d5e0d5ee690ba8a7135d2
GetErrorReason()'s Win32 implementation does the same thing as
Win32ErrorString(int err) from syserror.cpp, so call the latter.
Also remove now-unnecessary headers from sock.cpp and less verbose
handling of #ifdefs.
Only raw errno codes are logged if FileCommit fails. These are
implementation-specific, so it makes it harder to debug based on
user reports. Instead, use SysErrorString to display both the
raw int value and the descriptive message.
This change helps generalize shutdown code so an interrupt can be
provided to libbitcoinkernel callers. This may also be useful to
eventually de-globalize all of the shutdown code.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
After initially being merged in #20487, it's no-longer clear that an
internal syscall sandboxing mechanism is something that Bitcoin Core
should have/maintain, especially when compared to better
maintained/supported alterantives, i.e firejail.
Note that given where it's used, the sandbox also gets dragged into the
kernel.
There is some related discussion in #24771.
This should not require any sort of deprecation, as this was only ever
an opt-in, experimental feature.
Closes#24771.
The return type of TranslateArg is std::string, which creates a copy.
Fix this by moving everything into a lambda that takes a reference and
returns a reference.
Also, the format function is called without specifying the namespace it
lives in. Fix this by specifying the namespace. See also:
7a59865793/doc/developer-notes.md (L117-L137).
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from code that is not strictly required by it.
The settings code belongs into the common library and namespace, since
the kernel library should not depend on it. See doc/design/libraries.md
for more information on this rationale.
Changing the namespace of the moved functions is scripted in the
following commit.
7d3b35004b039f2bd606bb46a540de7babdbc41e refactor: Move system from util to common library (TheCharlatan)
7eee356c0a7fefd70c8de21689efa335f52a69ba refactor: Split util::AnyPtr into its own file (TheCharlatan)
44de325d95447498036479c3112ba741caf45bf6 refactor: Split util::insert into its own file (TheCharlatan)
9ec5da36b62276ae22e348f26f88aaf646357d6d refactor: Move ScheduleBatchPriority to its own file (TheCharlatan)
f871c69191dfe1331861ebcdbadb6bd47e45c8b1 kernel: Add warning method to notifications (TheCharlatan)
4452707edec91c7d7991f486dd41ef3edb4f7fbf kernel: Add progress method to notifications (TheCharlatan)
84d71457e7250ab25c0a11d1ad1c7657197ffd90 kernel: Add headerTip method to notifications (TheCharlatan)
447761c8228d58f948aae7e73ed079c028cacb97 kernel: Add notification interface (TheCharlatan)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel".
---
It removes the kernel library's dependency on `util/system` and `interface_ui`. `util/system` contains networking and shell-related code that should not be part of the kernel library. The following pull requests prepared `util/system` for this final step: https://github.com/bitcoin/bitcoin/pull/27419https://github.com/bitcoin/bitcoin/pull/27254https://github.com/bitcoin/bitcoin/pull/27238.
`interface_ui` defines functions for a more general node interface and has a dependency on `boost/signals2`. After applying the patches from this pull request, the kernel's reliance on boost is down to `boost::multiindex`.
The approach implemented here introduces some indirection, which makes the code a bit harder to read. Any suggestions for improving or reworking this pull request to make it more concise, or even reworking it into a more proper interface, are appreciated.
ACKs for top commit:
MarcoFalke:
re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e (no change) 🎋
stickies-v:
Code Review ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e
hebasto:
re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review.
Tree-SHA512: c8cfc698dc9d78e20191c444708f2d957501229abe95e5806106d1126fb9c5fbcee686fb55645658c0107ce71f10646f37a2fdf7fde16bbf22cbf1ac885dd08d
