Skip to content

Commit c43154d

Browse files
committed
Merge #1998: refactor!: Box the changeset of StoreErrorWithDump
418753f refactor!: `Box` changeset in `StoreErrorWithDump` (codingp110) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description Fixes #1991. As the linked issue mentions `Box`ing helps reduce memory overhead and [limits the size of enums with `StoreErrorWithDump` as variant](bitcoindevkit/bdk_wallet#277 (comment)). Breaking: The enum `StoreErrorWithDump` and the functions `load`,`dump` and `load_or_create` have been changed. ### Changelog Notice ``` Changed - `changeset` field of `StoreErrorWithDump` is now of type `Option<Box<C>>` ``` ### Checklists #### All Submissions: * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) ACKs for top commit: evanlinjin: ACK 418753f Tree-SHA512: c09d2b14989f62613283b728cd8f0eec849a2f3980b07e4289d55289a2834b27c32ff0fcb89706c636441c02b385fbb4a4175e93615f6e16fa2e2e0cfb0e4e26
2 parents 3c57999 + 418753f commit c43154d

File tree

1 file changed

+16
-12
lines changed

1 file changed

+16
-12
lines changed

crates/file_store/src/store.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ where
118118
/// let mut new_store =
119119
/// Store::create(&MAGIC_BYTES, &new_file_path).expect("must create new file");
120120
/// if let Some(aggregated_changeset) = changeset {
121-
/// new_store.append(&aggregated_changeset)?;
121+
/// new_store.append(aggregated_changeset.as_ref())?;
122122
/// }
123123
/// // The following will overwrite the original file. You will loose the corrupted
124124
/// // portion of the original file forever.
@@ -152,7 +152,7 @@ where
152152
f.read_exact(&mut magic_buf)?;
153153
if magic_buf != magic {
154154
return Err(StoreErrorWithDump {
155-
changeset: Option::<C>::None,
155+
changeset: Option::<Box<C>>::None,
156156
error: StoreError::InvalidMagicBytes {
157157
got: magic_buf,
158158
expected: magic.to_vec(),
@@ -194,7 +194,7 @@ where
194194
Ok(aggregated_changeset)
195195
}
196196
Err(iter_error) => Err(StoreErrorWithDump {
197-
changeset: aggregated_changeset,
197+
changeset: aggregated_changeset.map(Box::new),
198198
error: iter_error,
199199
}),
200200
},
@@ -220,7 +220,7 @@ where
220220
Self::create(magic, file_path)
221221
.map(|store| (store, Option::<C>::None))
222222
.map_err(|err: StoreError| StoreErrorWithDump {
223-
changeset: Option::<C>::None,
223+
changeset: Option::<Box<C>>::None,
224224
error: err,
225225
})
226226
}
@@ -257,7 +257,7 @@ where
257257
#[derive(Debug)]
258258
pub struct StoreErrorWithDump<C> {
259259
/// The partially-aggregated changeset.
260-
pub changeset: Option<C>,
260+
pub changeset: Option<Box<C>>,
261261

262262
/// The [`StoreError`]
263263
pub error: StoreError,
@@ -266,7 +266,7 @@ pub struct StoreErrorWithDump<C> {
266266
impl<C> From<io::Error> for StoreErrorWithDump<C> {
267267
fn from(value: io::Error) -> Self {
268268
Self {
269-
changeset: Option::<C>::None,
269+
changeset: Option::<Box<C>>::None,
270270
error: StoreError::Io(value),
271271
}
272272
}
@@ -371,7 +371,7 @@ mod test {
371371
changeset,
372372
error: StoreError::Bincode(_),
373373
}) => {
374-
assert_eq!(changeset, Some(test_changesets))
374+
assert_eq!(changeset, Some(Box::new(test_changesets)))
375375
}
376376
unexpected_res => panic!("unexpected result: {unexpected_res:?}"),
377377
}
@@ -399,7 +399,7 @@ mod test {
399399
changeset,
400400
error: StoreError::Bincode(_),
401401
}) => {
402-
assert_eq!(changeset, Some(test_changesets))
402+
assert_eq!(changeset, Some(Box::new(test_changesets)))
403403
}
404404
unexpected_res => panic!("unexpected result: {unexpected_res:?}"),
405405
}
@@ -500,10 +500,14 @@ mod test {
500500
.expect_err("should fail to aggregate");
501501
assert_eq!(
502502
err.changeset,
503-
changesets.iter().cloned().reduce(|mut acc, cs| {
504-
Merge::merge(&mut acc, cs);
505-
acc
506-
}),
503+
changesets
504+
.iter()
505+
.cloned()
506+
.reduce(|mut acc, cs| {
507+
Merge::merge(&mut acc, cs);
508+
acc
509+
})
510+
.map(Box::new),
507511
"should recover all changesets that are written in full",
508512
);
509513
// Remove file and start again

0 commit comments

Comments
 (0)