Add TransactionError to node namespace and include it directly instead of
relying on indirect include through common/messages.h
This is a followup to a previous commit which moved the TransactionError enum.
These changes were done in a separate followup just to keep the previous commit
more minimal and easy to review.
Add separate PSBTError enum instead of reusing TransactionError enum for PSBT
operations, and drop unused error codes. The error codes returned by PSBT
operations and transaction broadcast functions mostly do not overlap, so using
an unified enum makes it harder to call any of these functions and know which
errors actually need to be handled.
Define PSBTError in the common library because PSBT functionality is
implemented in the common library and used by both the node (for rawtransaction
RPCs) and the wallet.
Move util/message to common/signmessage so it is named more clearly, and
because the util library is not supposed to depend on other libraries besides
the crypto library. The signmessage functions use CKey, CPubKey, PKHash, and
DecodeDestination functions in the consensus and common libraries.
Both RPC and GUI now render a useful error message instead of (silently) failing.
Replace bool with util::Result<void> to clarify that this either succeeds or returns an error message.
fa46cc22bc 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 fa46cc22bc
TheCharlatan:
lgtm ACK fa46cc22bc
Tree-SHA512: 96ba1c60356ce93954fe5c2a59045771c6d1516ad0d9dc436ef1800a1f1b0153f0d5fb78ca99d53ad54ba25fbce36962bdf1d4325aceedfc8154a61347a6a915
Update CheckPackageLimits to use util::Result to pass the error message
instead of out parameter.
Also update test to reflect the error message from `CTxMempool`
`CheckPackageLimits` output.
Coin serialize method segfaults if IsSpent condition is true. This caused
multiprocess code to segfault when serializing the Coin& output argument to of
the Node::getUnspentOutput method if the coin was not found. Segfault could be
triggered by double clicking and viewing transaction details in the GUI
transaction list.
Fix this by replacing Coin& output argument with optional<Coin> return value to
avoid trying to serializing spent coins.
099dbe4224 GUI: TransactionRecord: When time/index/etc match, sort send before receive (Luke Dashjr)
2d182f77cd Bugfix: Ignore ischange flag when we're not the sender (Luke Dashjr)
71fbdb7f40 GUI: Remove SendToSelf TransactionRecord type (Luke Dashjr)
f3fbe99fcf GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs (Luke Dashjr)
b9765ba1d6 GUI: TransactionRecord: Use "any from me" as the criteria for deciding whether a transaction is a send or receive (Luke Dashjr)
Pull request description:
Makes the GUI transaction list more like the RPC, and IMO clearer in general.
As a side effect, this also fixes the GUI entries when a transaction is a net profit to us, but some inputs were also from us.
Originally https://github.com/bitcoin/bitcoin/pull/15115
Has Concept ACKs from @*Empact @*jonasschnelli
ACKs for top commit:
hebasto:
ACK 099dbe4224.
Tree-SHA512: 7d581add2f59431aa019126d54232a1f15723def5147d7a1b672e9b6d525b6e5a944cc437701aa1bd5bd0fbe557a3d1f4b239337f42bdba4fe1d3960442d0e3b
CTxDestination is really our internal representation of an address and
doesn't really have anything to do with standard script types, so move
them to their own file.
