backport: Merge bitcoin#28587, 25634, 28056, 27866, 27905, #7356
Draft
vijaydasmp wants to merge 5 commits into
Draft
backport: Merge bitcoin#28587, 25634, 28056, 27866, 27905, #7356vijaydasmp wants to merge 5 commits into
vijaydasmp wants to merge 5 commits into
Conversation
c1e6c54 descriptors: disallow hybrid public keys (Pieter Wuille) Pull request description: Fixes bitcoin#28511 The descriptor documentation (`doc/descriptors.md`) and [BIP380](https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki) explicitly require that hex-encoded public keys start with 02 or 03 (compressed) or 04 (uncompressed). However, the current parsing/inference code permit 06 and 07 (hybrid) encoding as well. Fix this. ACKs for top commit: darosior: ACK c1e6c54 achow101: ACK c1e6c54 Tree-SHA512: 23b674fb420619b2536d12da10008bb87cf7bc0333ec59e618c0d02c3574b468cc71248475ece37f76658d743ef51e68566948e903bca79fda5f7d75416fea4d
…llet flag should be un/set cdba23d wallet: Document blank flag use in descriptor wallets (Ryan Ofsky) 4331020 wallet: Ensure that the blank wallet flag is unset after imports (Andrew Chow) e9379f1 rpc, wallet: Include information about blank flag (Andrew Chow) Pull request description: The `blank` wallet flag is used to indicate that the wallet intentionally does not have any keys, scripts, or descriptors, and it prevents the automatic generation of those things for such a wallet. Once the wallet contains any of those data, it is unnecessary, and possibly incorrect, to have `blank` set. This PR fixes a few places where this was not properly happening. It also adds a test for this unset behavior. ACKs for top commit: S3RK: reACK cdba23d ryanofsky: Code review ACK cdba23d. Only change since last review is dropping the commit which makes createwallet RPC set BLANK flag automatically when DISABLE_PRIVATE_KEYS flag is set Tree-SHA512: 85bc2a9754df0531575d5c8f4ad7e8f38dcd50083dc29b3283dacf56feae842e81f34654c5e1781f2dadb0560ff80e454bbc8ca3b2d1fab1b236499ae9abd7da
…o `template_request` f6a2619 Added `longpollid` and `data` params to `template_request` bitcoin#27998 (Rhythm Garg) Pull request description: This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan. I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters. This PR refers to the issue bitcoin#27998 ACKs for top commit: ItIsOHM: > tACK [f6a2619](bitcoin@f6a2619) russeree: tACK bitcoin@f6a2619 stickies-v: tACK f6a2619 Tree-SHA512: 6c592db59cb11b2d031ce5265c547fa296266278f6c25f96afe18a420e0d547f4d483e0f66de75d52c0c319ac1585f3558b9f70c12ef208c96ec96a51f786c6a
d8041d4 blockstorage: Return on fatal undo file flush error (TheCharlatan) f0207e0 blockstorage: Return on fatal block file flush error (TheCharlatan) 5671c15 blockstorage: Mark FindBlockPos as nodiscard (TheCharlatan) Pull request description: The goal of this PR is to establish that fatal blockstorage flush errors should be treated as errors at their call site. Prior to this patch `FlushBlockFile` may have failed without returning in `Chainstate::FlushStateToDisk`, leading to a potential write from `WriteBlockIndexDB` that may refer to a block that is not fully flushed to disk yet. By returning if either `FlushUndoFile` or `FlushBlockFile` fail, we ensure that no further write operations take place that may lead to an inconsistent database when crashing. Add `[[nodiscard]]` annotations to them such that they are not ignored in future. Functions that call either `FlushUndoFile` or `FlushBlockFile`, need to handle these extra abort cases properly. Since `Chainstate::FlushStateToDisk` already produces an abort error in case of `WriteBlockIndexDB` failing, no extra logic for functions calling `Chainstate::FlushStateToDisk` is required. Besides `Chainstate::FlushStateToDisk`, `FlushBlockFile` is also called by `FindBlockPos`, while `FlushUndoFile` is only called by `FlushBlockFile` and `WriteUndoDataForBlock`. For both these cases, the flush error is not further bubbled up. Instead, the error is logged and a comment is provided why bubbling up an error would be less desirable in these cases. --- This pull request is part of a larger effort towards improving the shutdown / abort / fatal error handling in validation code. It is a first step towards implementing proper fatal error return type enforcement similar as proposed by theuni in this pull request [comment](bitcoin#27711 (comment)). For ease of review of these critical changes, a first step would be checking that `AbortNode` leads to early and error-conveying returns at its call site. Further work for enforcing returns when `AbortNode` is called is done in bitcoin#27862. ACKs for top commit: stickies-v: re-ACK d8041d4 ryanofsky: Code review ACK d8041d4 Tree-SHA512: 47ade9b873b15e567c8f60ca538d5a0daf32163e1031be3212a3a45eb492b866664b225f2787c9e40f3e0c089140157d8fd1039abc00c7bdfeec1b52ecd7e219
…ndex e639364 validation: add missing insert to m_dirty_blockindex (Martin Zumsande) Pull request description: When the status of a block index is changed, we must add it to `m_dirty_blockindex` or the change might not get persisted to disk. This is missing from one spot in `FindMostWorkChain()`, where `BLOCK_FAILED_CHILD` is set. Since we have [code](https://github.com/bitcoin/bitcoin/blob/f0758d8a6696657269d9c057e7aa079ffa9e1c16/src/node/blockstorage.cpp#L284-L287) that later sets missing `BLOCK_FAILED_CHILD` during the next startup, I don't think that this can lead to bad block indexes in practice, but I still think it's worth fixing. ACKs for top commit: TheCharlatan: ACK e639364 stickies-v: ACK e639364 Tree-SHA512: a97af9c173e31b90b677a1f95de822e08078d78013de5fa5fe4c3bec06f45d6e1823b7694cdacb887d031329e4b4afc6a2003916e0ae131279dee71f43e1f478
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bitcoin back ports