Fixup to #14978, which introduced a bunch of unused enum values, such as UNKNOWN_ERROR, ERROR_COUNT and TRANSACTION_ERR_LAST. None of those have a meaning in the context of an enum class, where the compiler can infer if all cases have been covered in a switch-case.
Also, move the global ::maxTxFee back to the rpc caller, so it can be set on a per call basis (in the future).
38- * return true on success, false on error (and fills in `error`)
39+ * @param[in] highfee Reject txs with fees higher than this (if 0, accept any fee)
40+ * return error
41 */
42-bool BroadcastTransaction(CTransactionRef tx, uint256& txid, TransactionError& error, std::string& err_string, bool allowhighfees = false);
43+TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee);
NODISCARD?
25 * @param[in] bip32derivs whether to fill in bip32 derivation information if available
26- * return true on success, false on error (and fills in `error`)
27+ * return error
28 */
29-bool FillPSBT(const CWallet* pwallet,
30+TransactionError FillPSBT(const CWallet* pwallet,
NODISCARD?
Concept ACK
Nice cleanup!
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
utACK
The only thing I feel weird about here is the caller of BroadcastTransaction needing to know that there is a maxTxFee they are supposed to be using. Would it be possible to default that parameter to maxTxFee? (So that calls to BroadcastTransaction automatically obey the -maxtxfee command line flag if not overridden deliberately?)