Make ChannelMonitor serialization slightly more upgradable
#1045
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1045 +/- ##
==========================================
- Coverage 90.96% 90.95% -0.01%
==========================================
Files 64 64
Lines 32393 32447 +54
==========================================
+ Hits 29467 29513 +46
- Misses 2926 2934 +8
Continue to review full report at Codecov.
|
TheBlueMatt
force-pushed
the
2021-08-chanmon-ser-upgradability
branch
from
97ef299 to
3843210
Compare
TheBlueMatt
force-pushed
the
2021-08-chanmon-ser-upgradability
branch
from
3843210 to
975c454
Compare
| pub(crate) struct VecReadWrapper<T: Readable>(pub Vec<T>); | ||
| impl<T: Readable> Readable for VecReadWrapper<T> { | ||
| pub(crate) struct VecReadWrapper<T>(pub Vec<T>); | ||
| impl<T: MaybeReadable> Readable for VecReadWrapper<T> { |
There was a problem hiding this comment.
IIUC, this is not exercised at all for MaybeReadable types now but could be in the future. That way we don't need to write custom decoding for TLVs with a vec containing types implementing MaybeReadable.
There was a problem hiding this comment.
Not sure I understand the comment. Obviously this PR is a bit awkward because it is just commits from #1034, which does use all the code in it. I tried a few ways to do VecReadWrapper that I thought were cleaner but always ended up hitting the "you may implement MaybeReadable and Readable for this type causing duplicate implementations" stuff every time.
There was a problem hiding this comment.
No action necessary. Just pointing out this use case.
This makes it much simpler to deal with `MaybeReadable` types in `Vec`s in TLVs as we can transparently deal with them as `vec`, with the wrapper doing the Right Thing. This requires we implement `MaybeReadable` for all `Readable` which has some downstream implications, but nothing too bad.
This adds a new TLV-based enum serialization macro entitled `impl_writeable_tlv_based_enum_upgradable`. As the name implies, the new macro allows us to ignore odd-numbered variant entries. Because the new macro implements only `MaybeReadable` and not `Readable`, it is not applicable in many contexts, here only being added for the two `OnchainEvent` structs.
TheBlueMatt
force-pushed
the
2021-08-chanmon-ser-upgradability
branch
from
975c454 to
ee43975
Compare
|
Squashed with one super-minor tweak as suggested by jeff, will take after CI:
Right, its kinda used for several things now, but at least its consistent in that the type is "maybe readable or maybe none" |
This is the first two commits from #1034, which make the
ChannelMonitorserialization slightly more upgradable and include an additional macro for tlv-based enum serialization using theMaybeReadabletrait. I'd like some feedback about maybe including this in 0.0.100 to avoid additional serialization breakages in 0.0.101.