Move Channel's blocked monitor updates vec to an even TLV

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.

[TheBlueMatt]

In 9dfe42c,
ChannelMonitorUpdates were stored in Channel while they were being processed. Because it was possible (though highly unlikely, due to various locking likely blocking persistence) an update was in-flight (even synchronously) when a ChannelManager was persisted, the new updates were persisted via an odd TLV.

However, in 4041f08 these pending monitor updates were moved to ChannelManager, with appropriate handling there. Now the only ChannelMonitorUpdates which are stored in Channel are those which are explicitly blocked, which requires the async pipeline.

Because we don't support async monitor update users downgrading to 0.0.115 or lower, we move to persisting them via an even TLV. As the odd TLV storage has not yet been released, we can do so trivially.

Fixes#2317.

[valentinewallace]

Could you remind me why it was OK that this was odd before? Not quite getting the commit message explanation. Is it because a previous version would just end up force closing due to stale monitors, lacking the pending updates to fill in the gap?

Otherwise LGTM.

[TheBlueMatt]

I was previously worried about the field holding monitor updates which were in-flight doing a regular sync persistence and getting serialized. (a) I think that worry was overblown cause we should always be holding the global consistency lock anyway, and (b) its definitely not true now since we never store any monitor updates in the channel anymore unless they're blocked (which they wont be until 117), and the ones that are stored in-flight in the manager are in an even TLV cause they're always in the global consistency lock.