| Age | Commit message (Collapse) | Author |
|---|
|
fa46cc22bc696e6845915ae91d6b68e36bf4c242 Remove deprecated -rpcserialversion (MarcoFalke)
Pull request description:
The flag is problematic for many reasons:
* It is deprecated
* It is a global flag, requiring a restart to change, as opposed to a flag that can be set on each RPC invocation
* It may be hidden in config files by accident, hard to debug, causing LND crashes and bugs, see https://github.com/bitcoin/bitcoin/issues/28730#issuecomment-1780940868
* It makes performance improvements harder to implement: https://github.com/bitcoin/bitcoin/pull/17529#issuecomment-556082818
Fix all issues by removing it.
If there is a use-case, likely a per-RPC flag can be added, if needed.
ACKs for top commit:
ajtowns:
crACK fa46cc22bc696e6845915ae91d6b68e36bf4c242
TheCharlatan:
lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242
Tree-SHA512: 96ba1c60356ce93954fe5c2a59045771c6d1516ad0d9dc436ef1800a1f1b0153f0d5fb78ca99d53ad54ba25fbce36962bdf1d4325aceedfc8154a61347a6a915
|
|
|
|
This makes it harder to pass nullptr and cause issues such as
https://github.com/bitcoin/bitcoin/commit/dde7ac5c704688c8a9af29bd07e5ae8114824ce7
|
|
The crash would happen when querying a mempool transaction with verbosity=2, while pruning.
|
|
|
|
The remaining places are handled easier outside a scripted-diff.
-BEGIN VERIFY SCRIPT-
sed --regexp-extended -i 's/CDataStream ([0-9a-zA-Z_]+)\(SER_[A-Z]+, [A-Z_]+_VERSION\);/DataStream \1{};/g' $( git grep -l CDataStream)
sed -i 's/, CDataStream/, DataStream/g' src/wallet/walletdb.cpp
-END VERIFY SCRIPT-
|
|
|
|
|
|
|
|
If processed psbt is complete return hex encoded network
transaction in the output.
|
|
fixing typo
|
|
Since script/standard only contains things that are used by the Solver
and its callers, rename the files to script/solver.
|
|
1bce12acd3e271a7c88d9400b4e3a5645bc8a911 test: add test for `descriptorprocesspsbt` RPC (ishaanam)
fb2a3a70e860aa87fb7a21f6554ed9f3ce901e2d rpc: add descriptorprocesspsbt rpc (ishaanam)
Pull request description:
This PR implements an RPC called `descriptorprocesspsbt`. This RPC is based off of `walletprocesspsbt`, but instead of interacting with the wallet to update, sign and finalize a psbt, it instead accepts an array of output descriptors and uses that information along with information from the mempool, txindex, and the utxo set to do so. `utxoupdatepsbt` also updates a psbt in this manner, but doesn't sign or finalize it. Because of this overlap, a helper function that is added in this PR is called by both `utxoupdatepsbt` and `descriptorprocesspsbt`. Whether or not the helper function signs a psbt is dictated by if the HidingSigningProvider passed to it contains any private information. There is also a test added in this PR for this new RPC that uses p2wsh, p2wpkh, and legacy outputs.
Edit: see https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1228830963
ACKs for top commit:
achow101:
re-ACK 1bce12acd3e271a7c88d9400b4e3a5645bc8a911
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/25796/commits/1bce12acd3e271a7c88d9400b4e3a5645bc8a911
Tree-SHA512: e1d0334739943e71f2ee68b4db7637ebe725da62e7aa4be071f71c7196d2a5970a31ece96d91e372d34454cde8509e95ab0eebd2c8edb94f7d5a781a84f8fc5d
|
|
This is a commit in preparation for the next few commits. The functions
are moved to methods to avoid their re-declaration for the purpose of
passing in BlockManager options.
The functions that were now moved into the BlockManager should no longer
use the params as an argument, but instead use the member variable.
In the moved ReadBlockFromDisk and UndoReadFromDisk, change
the function signature to accept a reference to a CBlockIndex instead of
a raw pointer. The pointer is expected to be non-null, so reflect that
in the type.
To allow for the move of functions to BlockManager methods all call
sites require an instantiated BlockManager, or a callback to one.
|
|
This RPC can be the Updater, Signer, and optionally the Input
Finalizer for a psbt, and has no interaction with the Bitcoin
Core wallet.
|
|
in the txindex
6e9f8bb050785dbc754b6bb493aad9139908ef98 rpc, tests: in `utxoupdatepsbt` also look for the transaction in the txindex (ishaanam)
a5b4883fb43d01bfef1244df62c64bf8691ca63a rpc: extract psbt updating logic into ProcessPSBT (ishaanam)
Pull request description:
Previously the `utxoupdatepsbt` RPC, added in #13932, only updated the inputs spending segwit outputs with the `witness_utxo`, because the full transaction is needed for legacy inputs. Before this RPC looked for just the utxos in the utxo set and the mempool. This PR makes it so that if the user has txindex enabled, then the full transaction is looked for there for all inputs. If it is not found in the txindex or txindex isn't enabled, then the mempool is checked for the full transaction. If the transaction an input is spending from is still not found at that point, then the utxo set is searched and the inputs spending segwit outputs are updated with just the utxo.
ACKs for top commit:
achow101:
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98
Xekyo:
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98
Tree-SHA512: 078db3c37a1ecd5816d80a42e8bd1341e416d661f508fa5fce0f4e1249fefd7b92a0d45f44957781cb69d0953145ef096ecdd4545ada39062be27742402dac6f
|
|
Previously only the segwit utxos being spent by the psbt were looked for and
added to the psbt. Now, the full transaction corresponding to each of these
utxos (legacy and segwit) is looked for in the txindex and mempool and added
to the psbt. If txindex is disabled and the transaction is not in the mempool,
then we fall back to getting just the utxo (if segwit) from the utxo set.
|
|
|
|
Using a FillableSigningProvider results in decodescript being unable to
infer descriptors for scripts larger than 520 bytes. Using a
FlatSigningProvider resolves this.
|
|
The descriptor inference logic would previously always use a dummy
signing provider and would never analyze the witness script of a P2WSH
scriptPubKey.
Note even a valid Miniscript might not always be decodable from Script
without more contextual information (for instance the key preimage for a
pk_h).
|
|
`TxToJSON()` and `TxToUniv()` are only to be called when we want to
decode the transaction (i.e. its details) into JSON. If `TxVerbosity` is
`SHOW_TXID`, the function should not have been (and currently is not)
called in the first place.
There is no behaviour change, current logic simply assumes anything less
than `TxVerbosity::SHOW_DETAILS_AND_PREVOUT` equals
`TxVerbosity::SHOW_DETAILS`. With this change, the assumptions and
intent become more explicit.
|
|
-BEGIN VERIFY SCRIPT-
sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp
sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/)
-END VERIFY SCRIPT-
|
|
This function is called from utxoupdatepsbt and will be modified
in a following commit to allow for updating inputs with the
`non_witness_utxo` as well.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
getrawtransaction
f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 rpc: Return fee and prevout(s) to getrawtransaction (Douglas Chimento)
Pull request description:
Add fee response in BTC to getrawtransaction #23264
### For Reviewers
* Verbose arg is now an int
* Verbose = 2 includes a `fee` field and `prevout`
* [./test/functional/rpc_rawtransaction.py](./test/functional/rpc_rawtransaction.py) contains a new test to validate fields of new verbosity 2 (not the values)
```
bitcoin-cli -chain=test getrawtransaction 9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4 2 000000000000001d442e556146d5f2841d85150c200e8d8b8a4b5005b13878f6
```
```
"in_active_chain": true,
"txid": "9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4",
"hash": "7f23e3f3a0a256ddea1d35ffd43e9afdd67cc68389ef1a804bb20c76abd6863e",
....
"vin": [
{
"txid": "23fc75d6d74f6f97e225839af69ff36a612fe04db58a4414ec4828d1749a05a0",
"vout": 0,
"scriptSig": {
"asm": "",
"hex": ""
},
"prevout": {
"generated": false,
"height": 2099486,
"value": 0.00017764,
"scriptPubKey": {
"asm": "0 7846ce1ced3253d8bd43008db2ca364cc722f5a2",
"hex": "00147846ce1ced3253d8bd43008db2ca364cc722f5a2",
"address": "tb1q0prvu88dxffa302rqzxm9j3kfnrj9adzk49mlp",
"type": "witness_v0_keyhash"
}
},
"sequence": 4294967295
},
...
"fee": 0.00000762
}
```
ACKs for top commit:
achow101:
ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9
aureleoules:
ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9
hernanmarino:
re ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9
pablomartin4btc:
re-tACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9
Tree-SHA512: 591fdc285d74fa7803e04ad01c7b70bc20fac6b1369e7bd5b8e2cde9b750ea52d6c70d79225b74bef4f4bbc0fb960877778017184e146119da4a55f9593d1224
|
|
This makes the code more robust, see previous commit.
In general replacing isTrue with get_bool is not equivalent because
get_bool can throw exceptions, but in this case, exceptions won't happen
because of RPCTypeCheck() and isNull() checks in the preceding code.
|
|
* Add optional fee response in BTC to getrawtransaction
* Add optional prevout(s) response to getrawtransaction showing utxos being spent
* Add getrawtransaction_verbosity functional test to validate fields
|
|
output has a script path
9e386afb67bf8fa71b72f730da1695eeb11828cd tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow)
30ff25cf37eec4b09ab40424eb5d6a4a80410955 psbt: Only include m_tap_tree if it has scripts (Andrew Chow)
0577d423adda8e719d7611d03355680c8fbacab8 psbt: Change m_tap_tree to store just the tuples (Andrew Chow)
22c051ca70bae73e0430b05fb9d879591df27699 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow)
7df6e1bb77a96eac4fbcba424bbe780636b86650 psbt: Fix merging of m_tap_tree (Andrew Chow)
0652dc53b291bd295caff4093ec2854fd4b34645 [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin)
Pull request description:
PSBT_OUT_TAP_TREE should not be included for outputs that do not have such a tree. This should be disallowed during parsing, as well as prior to serialization when the field is populated during updating.
Also added some test cases.
Alternative to #25856
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/25858/commits/9e386afb67bf8fa71b72f730da1695eeb11828cd
darosior:
ACK 9e386afb67bf8fa71b72f730da1695eeb11828cd
Tree-SHA512: ce5c02a69752d176dbd967c1e8d30129b1905c8f186aeeef034576c1de82059271a1ee846bd040f5be4e66bb77ba711dcf14ac1e597c5707d7e7e2293f6cfefb
|
|
Instead of having an entire TaprootBuilder which may or may not be
complete, and could potentially have future changes that interact oddly
with taproot tree tuples, have m_tap_tree be just the tuples.
When needed in other a TaprootBuilder for actual use, the tuples will be
added to a a TaprootBuilder that, in the future, can take in whatever
other data is needed as well.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-
Co-authored-by: MacroFake <falke.marco@gmail.com>
|
|
ab3c06db1aed979847158505f3df1dcea9fd6c2b doc: Release notes for default RBF (Andrew Chow)
61d9149e7804e2cec8fecf4150837344322eb301 rpc: Default rbf enabled (Andrew Chow)
e3c33637bac7db8ae56ab497df10911fad773981 wallet: Enable -walletrbf by default (Andrew Chow)
Pull request description:
The GUI currently opts in to RBF by default, but RPCs do not, and `-walletrbf` is default disabled. This PR makes the default in those two places to also opt in.
The last time this was proposed (#9527), the primary objections were the novelty at the time, the inability to bump transactions, and the gui not having the option to disable rbf. In the 5 years since, RBF usage has steadily grown, with ~27% of txs opting in. The GUI has the option to enable/disable RBF, and is also defaulted to having it enabled. And we have the ability to bump RBF'd transactions in both the RPC and the GUI. So I think it makes sense to finally change the default to always opt in to RBF.
ACKs for top commit:
darosior:
reACK ab3c06db1aed979847158505f3df1dcea9fd6c2b
aureleoules:
ACK ab3c06db1aed979847158505f3df1dcea9fd6c2b.
glozow:
utACK ab3c06db1a
Tree-SHA512: 81b012c5033e270f86a87a6a196ccc549eb54b158eebf88e917cc6621d40d7bdcd1566b602688907dd5d364b95a557b29f97dce869cea512e339588262c027b6
|
|
|
|
|
|
|
|
|
|
This change eliminates memory usage spike when compiling with Visual
Studio 2022 (at least in Cirrus CI environment).
Easy to review using
`git diff --color-moved-ws=allow-indentation-change --color-moved=dimmed-zebra`
|
|
This change eliminates memory usage spike when compiling with Visual
Studio 2022 (at least in Cirrus CI environment).
Easy to review using
`git diff --color-moved-ws=allow-indentation-change --color-moved=dimmed-zebra`
|
|
|
|
fa1b76aeb064b315a3767a8f59836ca18aeb117e Do not call global Params() when chainman is in scope (MacroFake)
fa30234be81b6f49ae8150478a9255daa1611083 Do not pass CChainParams& to PeerManager::make (MacroFake)
fafe5c0ca2927642cbcec63ac73994737e1653d6 Do not pass CChainParams& to BlockAssembler constructor (MacroFake)
faf012b438b451dced785e7f031e07c0c55665e1 Do not pass Consensus::Params& to Chainstate helpers (MacroFake)
fa4ee53dca5ccf1b87f019f372ffc10528add943 Do not pass time getter to Chainstate helpers (MacroFake)
Pull request description:
It seems confusing to pass chain params, consensus params, or a time function around when it is not needed.
Fix this by:
* Inlining the passed time getter function. I don't see a use case why this should be mockable.
* Using `chainman.GetConsensus()` or `chainman.GetParams()`, where possible.
ACKs for top commit:
promag:
Code review ACK fa1b76aeb064b315a3767a8f59836ca18aeb117e.
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/25168/commits/fa1b76aeb064b315a3767a8f59836ca18aeb117e
Tree-SHA512: 1abff5cba4b4871d97f17dbcdf67bc9255ff21fa4150a79a74e39b28f0610eab3e7dee24d56872dd6e111f003b55e288958cdd467e6218368d896f191e4ec9cd
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's|\<get_int64\>|getInt<int64_t>|g' $(git grep -l get_int ':(exclude)src/univalue')
sed -i 's|\<get_int\>|getInt<int>|g' $(git grep -l get_int ':(exclude)src/univalue')
-END VERIFY SCRIPT-
|
|
|
|
|
|
Can be reviewed with --word-diff-regex=. --ignore-all-space
|
|
UNREACHABLE macros
|
|
Can be reviewed with
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
`getrawtransaction`
71038a151e3136c22449e1a5cb1f386e8474c32d rpc: Fix documentation assertion for `getrawtransaction` (laanwj)
Pull request description:
When `getrawtransaction` is successfully used on a coinbase transaction, there is an assertion error. This is very unlikely but happens in the `interface_usdt_utxocache.py` test in #24358.
This does the following:
- Add missing "coinbase" documentation.
- Synchronize documentation between `getrawtransaction` and `decoderawtransaction`, the two users of `TxToUniv` that have detailed documentation. `decodepsbt` and `getblock` also uses it but fortunately elides this block.
- Change "vout[].amount" to `STR_AMOUNT` for consistency.
- Add maintainer comment to keep the two places synchronized. It might be possible to get smarter with deduplication, but there are some extra fields that prevent the obvious way.
ACKs for top commit:
jonatack:
ACK 71038a151e3136c22449e1a5cb1f386e8474c32d
Tree-SHA512: 962236130455d805190ff9a5c971e4e25c17db35614a90ce340264ec953b0ad7fb814eb33ae430b5073955a8a350f72bdd67ba93e35f9c70e5175b836a767a35
|
|
|
|
|
