Before this change, a malicious taker could send the same
commitment with different case/formatting, and it would not
be rejected as reuse, which is the intended behaviour. The
commitment is defined by the H(P2) value, not the string.
Reported by @m0wer ; included feedback from @kristapsk and Copilot.
style: revert formatting
fix(deps): update fastbencode to use rust variant
style: new line
chore: fix typo
Co-authored-by: Marnix Croes <93143998+MarnixCroes@users.noreply.github.com>
chore(deps): bump fastbencode to v0.3.6
chore(deps): remove fastbencode rust extra
f3f4f0a4fb Multiple (batch) payment support in `direct_send()` (Kristaps Kaupe)
Pull request description:
Work towards #1012. Changes `direct_send()` to instead of single `amount` and `destination` to accept `dest_and_amounts` which is list of tuples of addresses and amounts instead. Haven't yet implemented and tested actual payments to multiple recipients, but tested that this doesn't break existing stuff.
Top commit has no ACKs.
Tree-SHA512: 02195a28d071c9537cb5297e63854ad2571e0ae9b5e06b850d6173c47d53caae953e9d7671ff861a6584a104d7a59da2293781d4440f7db4814f9b2fc4116c46
053d8a18f2 Implement mixdepth filtering for showutxos (Kristaps Kaupe)
Pull request description:
Resolves#1539. As usual, also added type hints here and there.
Top commit has no ACKs.
Tree-SHA512: a61ce96cb1c79e046c0d44289411029cae050f30349aafa886c4882d77addcd8ec84d5f8990328fb7257436779c6cca9b87e89376aedbeb3e9d61d0e6d31a4c7
935a734b93 Fix tests for Core v26 when user has no access to ~/.bitcoin/settings.json (Kristaps Kaupe)
Pull request description:
From v26 Bitcoin Core always checks it's datadir `settings.json` file, even if you do stuff like `-help` or `-version`. It's stupid, but it is what it is. This makes problems with multi-user setups, where `bitcoind` runs under one user (`bitcoin`) and JoinMarket under another (`joinmarket`), and `/home/joinmarket/.bitcoin` is just symlink to original Bitcoin datadir, with sole purpose that RPC auth cookie is accessible, but `settings.json`, created by `bitcoind` itself, is not group readable. That is setup used for RaspiBolt, but not only, it's setup that makes sense.
Without this, tests fail on such setups:
```
(jmvenv) joinmarket@odroid:~/git/joinmarket-clientserver$ ./test/run_tests.sh
~/git/joinmarket-clientserver ~/git/joinmarket-clientserver
miniircd.tar.gz: OK
Error: Settings file could not be read:
- /home/joinmarket/.bitcoin/settings.json. Please check permissions.
==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.3.0
rootdir: /home/joinmarket/git/joinmarket-clientserver, configfile: pyproject.toml, testpaths: test
plugins: cov-2.5.1
collected 421 items
test/jmbase/test_base_support.py
!!!!!!!!! _pytest.outcomes.Exit: Cannot setup tests, bitcoind failing.
Error: Settings file could not be read:
- /home/joinmarket/.bitcoin/settings.json. Please check permissions. !!!!!!!!!!
(jmvenv) joinmarket@odroid:~/git/joinmarket-clientserver$ ls -al /home/joinmarket/.bitcoin
lrwxrwxrwx 1 joinmarket joinmarket 13 Nov 22 2022 /home/joinmarket/.bitcoin -> /data/bitcoin
(jmvenv) joinmarket@odroid:~/git/joinmarket-clientserver$ ls -al /data/bitcoin
total 115448
drwxrwxr-x 15 bitcoin bitcoin 4096 Dec 30 11:11 .
drwxr-xr-x 18 root root 4096 Oct 21 11:21 ..
-rw-r----- 1 bitcoin bitcoin 75 Dec 7 06:20 .cookie
-rw------- 1 bitcoin bitcoin 0 Sep 27 2021 .lock
drwx------ 3 bitcoin bitcoin 4096 Dec 7 06:20 SatSale
-rw------- 1 bitcoin bitcoin 34 Sep 27 2021 banlist.json
-rw-r----- 1 bitcoin bitcoin 1238 Dec 7 06:20 bitcoin.conf
drwx------ 3 bitcoin bitcoin 237568 Dec 30 06:00 blocks
drwx------ 2 bitcoin bitcoin 167936 Dec 30 11:19 chainstate
-rw------- 1 bitcoin bitcoin 17409758 Dec 30 11:23 debug.log
drwx------ 2 bitcoin bitcoin 4096 Sep 12 2022 descriptors_test
-rw------- 1 bitcoin bitcoin 247985 Dec 30 11:22 fee_estimates.dat
-rw------- 1 bitcoin bitcoin 679 Sep 27 2021 i2p_private_key
drwx------ 4 bitcoin bitcoin 4096 Sep 12 2022 indexes
drwx------ 3 bitcoin bitcoin 4096 Dec 7 06:21 jm_v26_test
drwx------ 3 bitcoin bitcoin 4096 Dec 7 08:30 jm_wallet
drwx------ 3 bitcoin bitcoin 4096 Feb 17 2022 joinmarket
drwx------ 2 bitcoin bitcoin 4096 Nov 27 2021 joinmarket-old
-rw------- 1 bitcoin bitcoin 94860907 Dec 7 06:02 mempool.dat
-rw------- 1 bitcoin bitcoin 99 Dec 7 06:21 onion_v3_private_key
-rw------- 1 bitcoin bitcoin 5189788 Dec 30 11:11 peers.dat
-rw-rw-r-- 1 bitcoin bitcoin 1534 Nov 27 2021 rpcauth.py
-rw------- 1 bitcoin bitcoin 60 Dec 19 13:19 settings.json
drwx------ 3 bitcoin bitcoin 4096 Sep 16 2022 specterd452d801d8c61692
drwx------ 3 bitcoin bitcoin 4096 Sep 16 2022 specterd452d801d8c61692_hotstorage
drwx------ 3 bitcoin bitcoin 4096 Sep 16 2022 specterf42828a49e7c6fd0
drwx------ 3 bitcoin bitcoin 4096 Dec 7 06:20 test
```
Fix is to provide `-conf`/`-datadir` argument to `bitcoind -version` calls.
See also discussion in https://github.com/raspibolt/raspibolt/pull/1387.
Top commit has no ACKs.
Tree-SHA512: ccceb876e216c946173191ade4f065b413cdaeda9a5c750cf2bda85d76381f4c4484da5a552c07a2a4c25cba0c38ce722826a497467a0fee15b23f82971cabda
ef1d76e9db Allow cache purging at wallet initialization (Adam Gibson)
Pull request description:
Fixes#1614.
Prior to this commit, if data in the persisted cache in the wallet file were wrong (should be a very extraordinary case), then the joinmarket code would have to crash with a cache invalid warning. After this commit, in such an extraordinary case, the option exists to invalidate or remove the cache on startup, so that it can be rebuilt from scratch. This is done with a config var wallet_caching_disabled in the POLICY section.
ACKs for top commit:
kristapsk:
utACK ef1d76e9db
Tree-SHA512: dea2256c7246417ab17be0c89d6b64f49243f35204a8d316ab520f94451d149b3131c928be020f7152f99e0eb5f07843341bb48fca5aaef4c6517701a122b355
Fixes#1614.
Prior to this commit, if data in the persisted cache in the wallet file
were wrong (should be a very extraordinary case), then the joinmarket
code would have to crash with a cache invalid warning. After this
commit, in such an extraordinary case, the option exists to invalidate
or remove the cache on startup, so that it can be rebuilt from scratch.
This is done with a config var wallet_caching_disabled in the POLICY
section.
c3c10f1615 wallet: implement optional cache validation (Matt Whitlock)
5bc7eb4b8e wallet: add persistent cache, mapping path->(priv, pub, script, addr) (Matt Whitlock)
01ec2a4181 wallet: add _addr_map, paralleling _script_map (Matt Whitlock)
64f18bce18 get_imported_privkey_branch: use O(m+n) algorithm instead of O(m*n) (Matt Whitlock)
77f0194a37 wallet_utils: use new get_utxos_at_mixdepth method (Matt Whitlock)
184d76f7f7 wallet: add get_{balance,utxos}_at_mixdepth methods (Matt Whitlock)
fc1e00058b wallet_showutxos: use O(1) check for frozen instead of O(n) (Matt Whitlock)
b58ac679cb wallet: drop _get_addr_int_ext; replace with calls to get_new_addr (Matt Whitlock)
2c38a813fc wallet: delete redundant get_script and get_addr methods (Matt Whitlock)
574c29e899 wallet: hoist get_script_from_path default impl into BaseWallet (Matt Whitlock)
8245271d7f wallet: avoid IndexError in _is_my_bip32_path (Matt Whitlock)
48aec83d76 wallet: remove a dead store in get_index_cache_and_increment (Matt Whitlock)
Pull request description:
**Note:** Reviewing each commit individually will make more sense than trying to review the combined diff.
This PR implements several performance enhancements that take the CPU time to run `wallet-tool.py display` on my wallet down from ~44 minutes to ~11 seconds.
The most significant gains come from replacing an **O**(_m_*_n_) algorithm in `get_imported_privkey_branch` with a semantically equivalent **O**(_m_+_n_) algorithm and from adding a persistent cache for computed private keys, public keys, scripts, and addresses.
Below are some actual benchmarks on my wallet, which has 5 mixdepths, each having path indices reaching into the 4000s, and almost 700 imported private keys.
* 673fbfb9a5 `origin/master` (baseline)
```
user 44m3.618s
sys 0m6.375s
```
* 48aec83d76 `wallet`: remove a dead store in `get_index_cache_and_increment`
* fbb681a207be465fb53b43ac18a2b52c8a4a6323 `wallet`: add `get_{balance,utxos}_at_mixdepth` methods
* 75a970378579bb04f189e8d9eca22e5e2aadb0b4 `wallet_utils`: use new `get_utxos_at_mixdepth` method
```
user 42m14.464s
sys 0m3.355s
```
* 84966e628d510ddf0cadba170346ea926dc06000 `wallet_showutxos`: use **O**(1) check for frozen instead of **O**(_n_)
* 75c5a75468a6de88e64c4af7a8226c633d358fd5 `get_imported_privkey_branch`: use **O**(_m_+_n_) algorithm instead of **O**(_m_*_n_)
```
user 5m0.045s
sys 0m0.453s
```
* da8daf048369081d882fb591d50583559a2284f0 `wallet`: add `_addr_map`, paralleling `_script_map`
```
user 4m56.175s
sys 0m0.423s
```
* d8aa1afe6f0ec596bb133f594ae88cc2fffb6ad2 `wallet`: add persistent cache, mapping path->(priv, pub, script, addr)
```
user 1m42.272s
sys 0m0.471s
```
* After running another command to modify the wallet file so as to persist the cache, `wallet-tool.py display` now runs in:
```
user 0m11.141s
sys 0m0.225s
```
ACKs for top commit:
AdamISZ:
tACK https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1594/commits/c3c10f1615631c3cca4d3c6be88c9c875e961d37
Tree-SHA512: fdd20d436d8f16a1e4270011ad1ba4bf6393f876eb7413da30f75d5830249134911d5d93cab8051c0bf107c213d4cd46ba9614ae23eef4566f867ff1b912fc9b
Add a validate_cache parameter to the five principal caching methods:
- _get_key_from_path
- _get_keypair_from_path
- _get_pubkey_from_path
- get_script_from_path
- get_address_from_path
and to the five convenience methods that wrap the above:
- get_script
- get_addr
- script_to_addr
- get_new_script
- get_new_addr
The value of this new parameter defaults to False in all but the last
two methods, where we are willing to sacrifice speed for the sake of
extra confidence in the correctness of *new* scripts and addresses to
be used for new deposits and new transactions.
Deriving private keys from BIP32 paths, public keys from private keys,
scripts from public keys, and addresses from scripts are some of the
most CPU-intensive tasks the wallet performs. Once the wallet inevitably
accumulates thousands of used paths, startup times become painful due to
needing to re-derive these data items for every used path in the wallet
upon every startup. Introduce a persistent cache to avoid the need to
re-derive these items every time the wallet is opened.
Introduce _get_keypair_from_path and _get_pubkey_from_path methods to
allow cached public keys to be used rather than always deriving them on
the fly.
Change many code paths that were calling CPU-intensive methods of
BTCEngine so that instead they call _get_key_from_path,
_get_keypair_from_path, _get_pubkey_from_path, get_script_from_path,
and/or get_address_from_path, all of which can take advantage of the new
cache.
Hoist _populate_script_map from BIP32Wallet into BaseWallet, rename it
to _populate_maps, and have it populate the new _addr_map in addition to
the existing _script_map. Have the constructor of each concrete wallet
subclass pass to _populate_maps the paths it contributes. Additionally,
do not implement yield_known_paths by iterating over _script_map, but
rather have each wallet subclass contribute its own paths to the
generator returned by yield_known_paths.
Sometimes calling code is only interested in the balance or UTXOs at a
single mixdepth. In these cases, it is wasteful to get the balance or
UTXOs at all mixdepths, only to throw away the returned information
about all but the single mixdepth of interest. Implement new methods in
BaseWallet to get the balance or UTXOs at a single mixdepth.
Also, correct an apparent oversight due to apparently misplaced
indentation: the maxheight parameter of get_balance_by_mixdepth was
ignored unless the include_disabled parameter was passed as False. It
appears that the intention was for include_disabled and maxheight to be
independent filters on the returned information.
* Add support for testing against multiple Bitcoin Core versions, add
0.18.0 (oldest officially support for test suite) and 25.0 (latest
stable release). Previously we tested against hardcoded 0.19.1. IMO
testing against oldest supported and latest makes sense, if tests pass
on both, they should pass on releases inbetween too, unless there is
some behaviour affecting us changed twice.
* Update actions/cache from v2 to v3.
Also changed Core downloads from bitcoin.org to bitcoincore.org.
refactoring to improve readability and testing
added support for taking the fee from multiple owned outputs as needed
added unit tests
removed same mixdepth restriction for inputs
added option for specifying which output to deduct fees from
added additional test cases
refactoring: moved tx_vsize to jmbitcoin package
added a sanity check to ensure that the transaction id supplied belongs to our wallet
fixed a linting error
c4d9b9264a tests: make setup fixtures optional. (PulpCattel)
Pull request description:
Currently, in tests, we always setup `bitcoind` and `miniircd` (this happens because of `autouse=True` in fixtures).
A lot of test modules do not require these slow and expensive setups. (`miniircd` in particular is AFAICT only used in 1 test module `jmdaemon/test/test_irc_messaging.py`).
After #1427, we now have indipendent fixtures for each component, so we can tell `Pytest` specifically what we need.
This PR uses [pytest.mark.usefixtures()](https://docs.pytest.org/en/7.2.x/how-to/fixtures.html#use-fixtures-in-classes-and-modules-with-usefixtures) to specify fixtures usage at the test module level.
Useful so that fixtures like `setup_regtest_bitcoind` can run *only* if we are planning to invoke a test that requires them.
For example, it's now possible to run:
```
pytest jmbitcoin/
```
This will skip all setup initialization because no test module in `/jmbitcoin` asks for it.
When running the entire test suite, there should be no difference in behaviour.
To see "behind the scene” of how `Pytest` schedules a test run, without actually running it, there's the very useful option `--setup-plan`.
ACKs for top commit:
AdamISZ:
Nice change, thanks. tACK https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1432/commits/c4d9b9264a3f814a9185b6e22d31343c30040389
Tree-SHA512: 307d8a60a13cbbd9648efc489cb6da2ecf8e3d42772df09ec99a14fa822ad86ca53212364fd437c79ab718a8effe0de6b6810f6a8ee1df3ae1ad44181beec7a3
This allows running tests that do not require, e.g., bitcoind, without having to run these setup fixtures at all.
Signed-off-by: PulpCattel <PulpCattel@users.noreply.github.com>
The former was a stale test that we never got to a properly
working state, so we replace it with test/e2e-coinjoin-test.py
which does the same job more effectively, using the RPC-API.
Also removed the old file from parameters of run_tests script.
Also, exports JMMakerClientProtocol for custom directory node scripts
(stored in the custom-scripts repo).
Modify default config with 2 signet and mainnet directory nodes to
start.
Handles unreachable directory nodes with a human readable error and
adjusts connection timeouts to be realistic.
Changes wording in Qt notifications from "IRC" to message channel.
Updates docs, new directory node information.
