json_db: fix StoredDict.__delitem__() to work similarly to .pop()
What the developer wrote
Authored by SomberNight
json_db: fix StoredDict.__delitem__() to work similarly to .pop()
follow-up https://github.com/spesmilo/electrum/pull/10233 ("jsondb pointers")
What changed, and why it matters
This commit fixes a consistency bug in Electrum's internal JSON database. When a user or code deleted a nested dictionary using `del dict[key]`, the child dictionary was not properly told it had been removed from its parent. That meant changes made to the child after deletion could still leak into the wallet's pending database updates. The fix makes `del dict[key]` behave the same as `.pop(key)` by clearing the child's parent pointer. The included tests confirm both methods now behave identically.
Treat as a low-severity correctness fix. Review whether any wallet operations or plugins use `del` on StoredDict instances and could have produced stale patches; no immediate emergency response is indicated, but users should update to include this follow-up fix.
Security signals we found
Data-integrity bug in wallet storage layer
Inconsistent behavior between dict.pop() and del dict[key]
Potential for stale nested objects to emit unintended database patches
Follow-up to prior JSON DB pointer refactor (PR #10233)
Evidence from the diff
StoredDict.delitem() in electrum/json_db.py now mirrors StoredDict.pop(): after removing the key, if the removed value is itself a StoredDict, its _parent reference is set to None. This prevents orphaned child StoredDicts from continuing to enqueue db_remove/db_add operations via their parent chain. The test suite was refactored to run the same assertions for both pop() and del-based removal, including nested removal cases.
Changed components
electrum/json_db.py:StoredDict.__delitem__()electrum/json_db.py:StoredDict.pop()tests/test_jsondb.pyInspect captured patch +44 / −26
diff --git a/electrum/json_db.py b/electrum/json_db.py
index 144a958..e0c0ecb 100644
--- a/electrum/json_db.py
+++ b/electrum/json_db.py
@@ -222,8 +222,11 @@ class StoredDict(dict, BaseStoredObject):
@locked
def __delitem__(self, key: _FLEX_KEY) -> None:
assert isinstance(key, _FLEX_KEY), repr(key)
+ r = self.get(key, None)
dict.__delitem__(self, key)
self.db_remove(key)
+ if isinstance(r, StoredDict):
+ r._parent = None
@locked
def pop(self, key: _FLEX_KEY, v=_RaiseKeyError) -> Any:
diff --git a/tests/test_jsondb.py b/tests/test_jsondb.py
index f0cf30b..bb07eaf 100644
--- a/tests/test_jsondb.py
+++ b/tests/test_jsondb.py
@@ -2,6 +2,7 @@ import contextlib
import copy
import traceback
import json
+from typing import Any
import jsonpatch
from jsonpatch import JsonPatchException
@@ -88,6 +89,16 @@ class TestJsonpatch(ElectrumTestCase):
fail_if_leaking_secret(ctx)
+def pop1_from_dict(d: dict, key: str) -> Any:
+ return d.pop(key)
+
+
+def pop2_from_dict(d: dict, key: str) -> Any:
+ val = d[key]
+ del d[key]
+ return val
+
+
class TestJsonDB(ElectrumTestCase):
async def test_jsonpatch_replace_after_remove(self):
@@ -109,31 +120,35 @@ class TestJsonDB(ElectrumTestCase):
data = jpatch.apply(data)
async def test_jsondb_replace_after_remove(self):
- data = { 'a': {'b': {'c': 0}}, 'd': 3}
- db = JsonDB(repr(data))
- a = db.get_dict('a')
- # remove
- b = a.pop('b')
- self.assertEqual(len(db.pending_changes), 1)
- # replace item. this must not been written to db
- b['c'] = 42
- self.assertEqual(len(db.pending_changes), 1)
- patches = json.loads('[' + ','.join(db.pending_changes) + ']')
- jpatch = jsonpatch.JsonPatch(patches)
- data = jpatch.apply(data)
- self.assertEqual(data, {'a': {}, 'd': 3})
+ for pop_from_dict in [pop1_from_dict, pop2_from_dict]:
+ with self.subTest(pop_from_dict):
+ data = { 'a': {'b': {'c': 0}}, 'd': 3}
+ db = JsonDB(repr(data))
+ a = db.get_dict('a')
+ # remove
+ b = pop_from_dict(a, 'b')
+ self.assertEqual(len(db.pending_changes), 1)
+ # replace item. this must not been written to db
+ b['c'] = 42
+ self.assertEqual(len(db.pending_changes), 1)
+ patches = json.loads('[' + ','.join(db.pending_changes) + ']')
+ jpatch = jsonpatch.JsonPatch(patches)
+ data = jpatch.apply(data)
+ self.assertEqual(data, {'a': {}, 'd': 3})
async def test_jsondb_replace_after_remove_nested(self):
- data = { 'a': {'b': {'c': 0}}, 'd': 3}
- db = JsonDB(repr(data))
- # remove
- a = db.data.pop('a')
- self.assertEqual(len(db.pending_changes), 1)
- b = a['b']
- # replace item. this must not be written to db
- b['c'] = 42
- self.assertEqual(len(db.pending_changes), 1)
- patches = json.loads('[' + ','.join(db.pending_changes) + ']')
- jpatch = jsonpatch.JsonPatch(patches)
- data = jpatch.apply(data)
- self.assertEqual(data, {'d': 3})
+ for pop_from_dict in [pop1_from_dict, pop2_from_dict]:
+ with self.subTest(pop_from_dict):
+ data = { 'a': {'b': {'c': 0}}, 'd': 3}
+ db = JsonDB(repr(data))
+ # remove
+ a = pop_from_dict(db.data, "a")
+ self.assertEqual(len(db.pending_changes), 1)
+ b = a['b']
+ # replace item. this must not be written to db
+ b['c'] = 42
+ self.assertEqual(len(db.pending_changes), 1)
+ patches = json.loads('[' + ','.join(db.pending_changes) + ']')
+ jpatch = jsonpatch.JsonPatch(patches)
+ data = jpatch.apply(data)
+ self.assertEqual(data, {'d': 3})
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.