Skip to content

Commit d885ecd

Browse files
knstPastaPastaPasta
authored andcommitted
8cfc3c4 Merge bitcoin#27695: test: Add test to check tx in the last block can be downloaded (DCG-Claude) 3e2077c Merge bitcoin#28232: test: locked_wallet, skip default fee estimation (DCG-Claude) a579bef Merge bitcoin#28125: wallet: bugfix, disallow migration of invalid scripts (DCG-Claude) 2d7b155 Merge bitcoin#27800: streams: Drop confusing DataStream::Serialize method and << operator (DCG-Claude) Pull request description: <!-- backportsys v1 --> > 🔎 **Needs a careful look.** Commits are ordered by how much they were adapted: 🔴 heavy first, then 🟡 light (at most two invented lines, or up to six lines counting edits of upstream functional-test or doc lines), then 🟢 faithful (identical to upstream apart from context). ## Issue being fixed or feature implemented Backports 4 Bitcoin Core v0.26 pull request(s) that the Dash queue selected, including any discovered prerequisites: bitcoin#27800, bitcoin#28125, bitcoin#28232, bitcoin#27695. ## What was done? | upstream | commit | review | gates | notes | |---|---|---|---|---| | bitcoin#27800 | `2d7b155784` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#27800: removes DataStream::Serialize and updates the upstream call sites in wallet/dump.cpp | | bitcoin#28232 | `3e2077c380` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#28232: the locked_wallet test now sets a fixed tx fee so it no longer depends on fee estimat | | bitcoin#27695 | `8cfc3c4522` | 🔴 heavy | build:pass ci_upstream:warn mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#27695 in full: p2p_leak_tx.py is split into test_tx_in_block and test_notfound_on_unannounce | | bitcoin#28125 | `a579befd06` | 🟡 light | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#28125 in full: legacy-wallet migration now skips invalid or non-watched watch-only scripts ( | Each commit keeps the upstream subject (`partial Merge …` only where a hunk is deferred to a prerequisite still to be backported, named in the commit body; a hunk Dash intentionally never wants is recorded in the commit body or the reviewer's note above and does not make the backport partial). Conflicts were resolved commit by commit; commits that needed no resolution were cherry-picked unchanged. ## How Has This Been Tested? Recorded per commit, at that commit's own sha, not once for the branch: - built at each commit, reusing the worktree build cache - the test suites touched by each diff run at that commit, where the diff selected one - a mechanical diff-of-diffs against the upstream patch (every upstream hunk present, no added line without an upstream counterpart, no Dash-specific line dropped) - an independent verification pass on 4 commit(s) that needed adaptation - fork gate PR thepastaclaw#100 at `8cfc3c4522` — **no passing CI record at this head** Gates that did not come back clean — please weigh these: - bitcoin#27800 `mech`: **warn** — 9 invented line(s); 1 Dash-feature line(s) removed; 1 partial/prereq/low-risk finding(s) - bitcoin#28125 `mech`: **warn** — 1 edited line(s) - bitcoin#28232 `mech`: **warn** — 1 invented line(s); 1 partial/prereq/low-risk finding(s) - bitcoin#27695 `mech`: **warn** — 5 invented line(s); 1 partial/prereq/low-risk finding(s) ## Breaking Changes None beyond the upstream changes themselves. ## Checklist: _Left for the reviewer; backportsys does not tick boxes on its own behalf._ - [ ] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- ### Maintainer controls <!-- backportsys controls --> Tick a box and backportsys acts on it within a few minutes, then clears the box. For anything else — a hunk to drop, a resolution to redo, a question — just leave a review comment; nothing here needs a box. - [ ] 🔒 **Hands off** — stop every automated update to this branch <!-- bps:hands_off --> - [ ] 🔄 **Rebase** onto current `develop` <!-- bps:rebase --> - [ ] ❌ **Close** — abandon this batch and release its items <!-- bps:close --> <!-- /backportsys controls --> <!-- human notes below; the bot never edits this section --> ACKs for top commit: knst: utACK 8cfc3c4 Tree-SHA512: da34d35ae7071be1d25f9bf7e8b06f94e3f81f50c40d1c278230be3af6ee37d74e86417be89409911dffc4b990eb5c35c704ffaa09d51847b6abfc307180be5f (cherry picked from commit fe232f9)
1 parent f55ad1d commit d885ecd

15 files changed

Lines changed: 112 additions & 36 deletions

‎src/flat-database.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class CFlatDB
5959

6060
// Write and commit header, data
6161
try {
62-
fileout << ssObj;
62+
fileout << Span{ssObj};
6363
}
6464
catch (std::exception &e) {
6565
LogError("%s: Serialize or I/O error - %s\n", __func__, e.what());

‎src/governance/net_governance.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,15 @@ bool NetGovernance::ProcessGetData(CNode& pfrom, const CInv& inv, const CNetMsgM
288288
CDataStream ss(SER_NETWORK, pfrom.GetCommonVersion());
289289
ss.reserve(1000);
290290
if (!m_gov_manager.SerializeObjectForHash(inv.hash, ss)) return false;
291-
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, ss));
291+
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, Span{ss}));
292292
return true;
293293
}
294294
if (inv.type == MSG_GOVERNANCE_OBJECT_VOTE) {
295295
if (!m_gov_manager.HaveVoteForHash(inv.hash)) return false;
296296
CDataStream ss(SER_NETWORK, pfrom.GetCommonVersion());
297297
ss.reserve(1000);
298298
if (!m_gov_manager.SerializeVoteForHash(inv.hash, ss)) return false;
299-
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, ss));
299+
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, Span{ss}));
300300
return true;
301301
}
302302
return false;

‎src/llmq/dkgsessionmgr.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqT
129129
for (auto& pubkey : *vvec) {
130130
s << CBLSPublicKeyVersionWrapper(pubkey, false);
131131
}
132-
db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), s);
132+
db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), Span{s});
133133
}
134134

135135
void CDKGSessionManager::WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution)

‎src/llmq/net_quorum.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS
113113
}
114114
request.SetError(nError);
115115
CDataStream ssResponse{SER_NETWORK, pfrom.GetCommonVersion()};
116-
ssResponse << request << body;
117-
m_connman.PushMessage(&pfrom, CNetMsgMaker(pfrom.GetCommonVersion()).Make(NetMsgType::QDATA, ssResponse));
116+
ssResponse << request << Span{body};
117+
m_connman.PushMessage(&pfrom, CNetMsgMaker(pfrom.GetCommonVersion()).Make(NetMsgType::QDATA, Span{ssResponse}));
118118
return misbehave;
119119
};
120120

‎src/llmq/quorums.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ void CQuorum::WriteContributions(CDBWrapper& db) const
200200
for (auto& pubkey : *quorumVvec) {
201201
s << CBLSPublicKeyVersionWrapper(pubkey, false);
202202
}
203-
db.Write(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), s);
203+
db.Write(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), Span{s});
204204
}
205205
if (skShare.IsValid()) {
206206
db.Write(std::make_pair(DB_QUORUM_SK_SHARE, dbKey), skShare);

‎src/llmq/quorumsman.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ void CQuorumManager::MigrateOldQuorumDB(CEvoDB& evoDb) const
676676
if (!evoDb.GetRawDB().ReadDataStream(k, s)) {
677677
break;
678678
}
679-
batch.Write(k, s);
679+
batch.Write(k, Span{s});
680680
}
681681
if (prefix == DB_QUORUM_SK_SHARE) {
682682
if (!pcursor->GetValue(sk)) {

‎src/streams.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,14 +316,6 @@ class DataStream
316316
vch.insert(vch.end(), src.begin(), src.end());
317317
}
318318

319-
template<typename Stream>
320-
void Serialize(Stream& s) const
321-
{
322-
// Special case: stream << stream concatenates like stream += stream
323-
if (!vch.empty())
324-
s.write(MakeByteSpan(vch));
325-
}
326-
327319
template<typename T>
328320
DataStream& operator<<(const T& obj)
329321
{

‎src/wallet/dump.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::
251251

252252
std::vector<unsigned char> k = ParseHex(key);
253253
std::vector<unsigned char> v = ParseHex(value);
254-
255-
DataStream ss_key{k};
256-
DataStream ss_value{v};
257-
258-
if (!batch->Write(ss_key, ss_value)) {
254+
if (!batch->Write(Span{k}, Span{v})) {
259255
error = strprintf(_("Error: Unable to write record to new wallet"));
260256
ret = false;
261257
break;

‎src/wallet/scriptpubkeyman.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1872,8 +1872,23 @@ std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPub
18721872
}
18731873

18741874
// All watchonly scripts are raw
1875-
spks.insert(setWatchOnly.begin(), setWatchOnly.end());
1875+
for (const CScript& script : setWatchOnly) {
1876+
// As the legacy wallet allowed to import any script, we need to verify the validity here.
1877+
// LegacyScriptPubKeyMan::IsMine() return 'ISMINE_NO' for invalid or not watched scripts (IsMineResult::INVALID or IsMineResult::NO).
1878+
// e.g. a "sh(sh(pkh()))" which legacy wallets allowed to import!.
1879+
if (IsMine(script) != ISMINE_NO) spks.insert(script);
1880+
}
1881+
1882+
return spks;
1883+
}
18761884

1885+
std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetNotMineScriptPubKeys() const
1886+
{
1887+
LOCK(cs_KeyStore);
1888+
std::unordered_set<CScript, SaltedSipHasher> spks;
1889+
for (const CScript& script : setWatchOnly) {
1890+
if (IsMine(script) == ISMINE_NO) spks.insert(script);
1891+
}
18771892
return spks;
18781893
}
18791894

‎src/wallet/scriptpubkeyman.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,12 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
526526
std::set<CKeyID> GetKeys() const override;
527527
std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override;
528528

529+
/**
530+
* Retrieves scripts that were imported by bugs into the legacy spkm and are
531+
* simply invalid, such as a sh(sh(pkh())) script, or not watched.
532+
*/
533+
std::unordered_set<CScript, SaltedSipHasher> GetNotMineScriptPubKeys() const;
534+
529535
/** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan.
530536
* Does not modify this ScriptPubKeyMan. */
531537
std::optional<MigrationData> MigrateToDescriptor();

0 commit comments

Comments
 (0)