Breakup CooperativeClosure into Local/Remote initiated #2863
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe update introduces more granular distinctions for cooperative closures in the Lightning Network's codebase. Previously lumped together as Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
benthecarman
force-pushed
the
breakup-coop-close
branch
2 times, most recently
from
1aa85d8 to
b16a4b2
Compare
|
A good nights sleep let me figure this out :) |
![coderabbitai[bot]](/~https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- lightning/src/events/mod.rs (3 hunks)
- lightning/src/ln/chanmon_update_fail_tests.rs (2 hunks)
- lightning/src/ln/channel.rs (9 hunks)
- lightning/src/ln/channelmanager.rs (1 hunks)
- lightning/src/ln/functional_tests.rs (4 hunks)
- lightning/src/ln/monitor_tests.rs (1 hunks)
- lightning/src/ln/shutdown_tests.rs (13 hunks)
Files not reviewed due to errors (2)
- lightning/src/ln/monitor_tests.rs (Error: unable to parse review)
- lightning/src/ln/chanmon_update_fail_tests.rs (Error: unable to parse review)
Additional comments: 34
lightning/src/ln/shutdown_tests.rs (14)
- 71-72: The changes to the
check_closed_event!macro calls correctly reflect the updated closure reasons, aligning with the PR's objective to distinguish between locally and counterparty initiated cooperative closures.- 116-117: The updates to the
check_closed_event!macro calls are consistent with the intended changes, accurately representing the new closure reason variants.- 199-200: The modifications to the
check_closed_event!macro calls correctly implement the new closure reason variants, ensuring the test logic matches the updated channel shutdown behavior.- 254-255: The changes to the
check_closed_event!macro calls are appropriate and reflect the new closure reason variants, aligning with the PR's goals.- 407-408: The updates to the
check_closed_event!macro calls accurately reflect the new closure reason variants, ensuring the test logic is consistent with the updated channel shutdown behavior.- 417-418: The modifications to the
check_closed_event!macro calls correctly implement the new closure reason variants, ensuring the test logic matches the updated channel shutdown behavior.- 512-514: The changes to the
check_closed_event!macro calls are appropriate and reflect the new closure reason variants, aligning with the PR's goals.- 655-655: The use of
check_closed_event!macro calls with the updated closure reasons correctly reflects the intended changes, ensuring the test logic is consistent with the updated channel shutdown behavior.- 695-697: The modifications to the
check_closed_event!macro calls correctly implement the new closure reason variants, ensuring the test logic matches the updated channel shutdown behavior.- 1138-1138: The use of
check_closed_event!macro calls with the updated closure reasons correctly reflects the intended changes, ensuring the test logic is consistent with the updated channel shutdown behavior.- 1140-1140: The modifications to the
check_closed_event!macro calls correctly implement the new closure reason variants, ensuring the test logic matches the updated channel shutdown behavior.- 1224-1225: The changes to the
check_closed_event!macro calls are appropriate and reflect the new closure reason variants, aligning with the PR's goals.- 1279-1280: The updates to the
check_closed_event!macro calls accurately reflect the new closure reason variants, ensuring the test logic is consistent with the updated channel shutdown behavior.- 1372-1373: The modifications to the
check_closed_event!macro calls correctly implement the new closure reason variants, ensuring the test logic matches the updated channel shutdown behavior.lightning/src/events/mod.rs (6)
- 184-197: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-52]
The
PaymentPurposeenum is well-defined, providing clear differentiation between invoice payments and spontaneous payments. This distinction is essential for correctly handling payments based on their origin and intent.
- 184-197: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [53-59]
The
impl_writeable_tlv_based_enum!macro usage forPaymentPurposecorrectly matches the enum's structure, ensuring proper serialization and deserialization of the enum variants. This is crucial for maintaining data integrity across the network.
- 184-197: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [187-268]
The
ClosureReasonenum comprehensively covers all possible reasons for channel closure, including cooperative closures initiated by either the local node or the counterparty, as well as various error conditions. TheDisplayimplementation provides meaningful descriptions for each variant, aiding in debugging and logging.
- 261-268: The
impl_writeable_tlv_based_enum_upgradable!macro usage forClosureReasonis correctly implemented, ensuring that each variant is properly serialized and deserialized. This is vital for maintaining the integrity of channel closure reasons across the network.- 184-197: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [187-268]
The
Eventenum comprehensively covers a wide range of events that can occur within the Lightning Network, including payment-related events, channel lifecycle events, and more. This coverage is crucial for enabling robust event handling in Lightning Network implementations.
- 184-197: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [187-268]
The
WriteableandMaybeReadabletrait implementations forEventare correctly implemented, ensuring proper serialization and deserialization of events. This is critical for maintaining data integrity and compatibility across different versions of the Lightning Network protocol.lightning/src/ln/functional_tests.rs (4)
- 874-875: The update to
check_closed_event!macro calls, specifyingLocallyInitiatedCooperativeClosureandCounterpartyInitiatedCooperativeClosureas closure reasons, aligns well with the PR's objectives to distinguish between closures initiated by the local node and those initiated by the remote counterparty. This change enhances the clarity and specificity of the test assertions.- 988-989: Similar to the previous comment, the modifications here correctly implement the PR's objective by using the new closure reason variants. This ensures that the tests accurately reflect the intended behavior of distinguishing between different initiators of cooperative closures.
- 1107-1117: The updates in this segment continue to accurately reflect the PR's goal by distinguishing between locally and counterparty initiated closures across multiple channel closure scenarios. The use of both
LocallyInitiatedCooperativeClosureandCounterpartyInitiatedCooperativeClosurein various contexts ensures comprehensive testing of the new logic.- 5622-5630: The modifications in this section correctly apply the new closure reason variants following channel closure and mining transactions. This ensures that the tests accurately assess the system's behavior in response to channel closures initiated by different parties, aligning with the PR's objectives.
lightning/src/ln/channel.rs (9)
- 350-351: The addition of
LOCAL_INITIATED_SHUTDOWNstate flag and its associated methods is correctly implemented. However, ensure that the flag's usage throughout the codebase is consistent and does not introduce any unintended side effects.- 393-393: The definition of
LOCAL_INITIATED_SHUTDOWNwith a unique bit value (1 << 14) is correct and follows the established pattern for state flags. Ensure that there's no overlap with other state flags to avoid unintended state mutations.- 413-415: The documentation comment for
LOCAL_INITIATED_SHUTDOWNcorrectly indicates its purpose. However, ensure that the documentation is comprehensive and accurately reflects the flag's role in the shutdown process.- 582-582: The implementation of state flag methods for
LOCAL_INITIATED_SHUTDOWNis consistent with the existing pattern. It's important to ensure that these methods are used appropriately in the shutdown logic to accurately reflect the shutdown initiator.- 1336-1336: The logic change to use
is_local_shutdown_sent()andis_remote_shutdown_sent()methods for determining if the channel is in a shutdown state is more readable and maintainable. Ensure that this change does not alter the intended logic of the shutdown process.- 4969-4974: The assignment of
closure_reasonbased oninitiated_shutdown()introduces clear differentiation between locally and counterparty initiated cooperative closures. This is a significant improvement for clarity and debugging. Ensure thatinitiated_shutdown()accurately reflects the shutdown initiation state in all relevant scenarios.- 5014-5014: Repeating the assignment of
closure_reasonin this context suggests that the logic is consistent across different parts of the shutdown process. However, ensure that this redundancy is necessary and that there isn't a more efficient way to handleclosure_reasonassignment.- 5280-5282: The
initiated_shutdown()method provides a clear and concise way to check if the shutdown was locally initiated. This method enhances the readability and maintainability of the shutdown logic. Ensure that it is used consistently in all relevant parts of the codebase.- 6198-6198: Setting
LOCAL_INITIATED_SHUTDOWNupon initiating a local shutdown is correct and ensures that the shutdown initiator is accurately tracked. This change is crucial for the logic that differentiates between locally and counterparty initiated closures. Ensure that this flag is set and cleared at appropriate times to accurately reflect the channel's state.lightning/src/ln/channelmanager.rs (1)
- 11859-11860: The updates to the
check_closed_event!calls, usingLocallyInitiatedCooperativeClosureandCounterpartyInitiatedCooperativeClosure, align well with the PR's objectives to distinguish between different initiators of cooperative closures. This change enhances the clarity of the code by making the closure reasons more specific.
wpaulino
previously approved these changes
Jan 31, 2024
shaavan
reviewed
Feb 1, 2024
Comment on lines
260
to
+262
| (6, CommitmentTxConfirmed) => {}, | ||
| (4, CooperativeClosure) => {}, | ||
| (4, LegacyCooperativeClosure) => {}, |
There was a problem hiding this comment.
Unrelated nit:
Since we are touching these lines, should we think about reordering these enums? I think it's pretty odd to write 6th before 4th.
Suggested change
| (6, CommitmentTxConfirmed) => {}, | |
| (4, CooperativeClosure) => {}, | |
| (4, LegacyCooperativeClosure) => {}, | |
| (4, LegacyCooperativeClosure) => {}, | |
| (6, CommitmentTxConfirmed) => {}, | |
benthecarman
force-pushed
the
breakup-coop-close
branch
from
b16a4b2 to
c99a60a
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- lightning/src/events/mod.rs (3 hunks)
- lightning/src/ln/chanmon_update_fail_tests.rs (2 hunks)
- lightning/src/ln/channel.rs (11 hunks)
- lightning/src/ln/channelmanager.rs (1 hunks)
- lightning/src/ln/functional_tests.rs (4 hunks)
- lightning/src/ln/monitor_tests.rs (1 hunks)
- lightning/src/ln/shutdown_tests.rs (13 hunks)
Files skipped from review as they are similar to previous changes (6)
- lightning/src/ln/chanmon_update_fail_tests.rs
- lightning/src/ln/channel.rs
- lightning/src/ln/channelmanager.rs
- lightning/src/ln/functional_tests.rs
- lightning/src/ln/monitor_tests.rs
- lightning/src/ln/shutdown_tests.rs
Additional comments: 3
lightning/src/events/mod.rs (3)
- 187-194: The addition of
LegacyCooperativeClosure,CounterpartyInitiatedCooperativeClosure, andLocallyInitiatedCooperativeClosurevariants to theClosureReasonenum is a significant improvement for distinguishing between different types of cooperative closures. This change enhances the clarity of the code by providing more specific reasons for channel closures, which can be useful for debugging and analytics.- 239-241: The implementation of the
Displaytrait for the newClosureReasonvariants is correctly done. It ensures that each variant is represented with a clear and descriptive message, which is essential for logging and user-facing messages. This is a good practice for maintainability and usability.- 261-268: The serialization of the new
ClosureReasonvariants in theimpl_writeable_tlv_based_enum_upgradable!macro is correctly implemented. This ensures that the new variants are correctly serialized and deserialized, maintaining compatibility and data integrity. It's important to ensure that these changes are thoroughly tested, especially for upgrades and downgrades between different versions of the software.
wpaulino
reviewed
Feb 1, 2024
benthecarman
force-pushed
the
breakup-coop-close
branch
from
c99a60a to
23c17f9
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- lightning/src/events/mod.rs (3 hunks)
- lightning/src/ln/chanmon_update_fail_tests.rs (2 hunks)
- lightning/src/ln/channel.rs (11 hunks)
- lightning/src/ln/channelmanager.rs (1 hunks)
- lightning/src/ln/functional_tests.rs (4 hunks)
- lightning/src/ln/monitor_tests.rs (1 hunks)
- lightning/src/ln/shutdown_tests.rs (13 hunks)
Files skipped from review as they are similar to previous changes (6)
- lightning/src/events/mod.rs
- lightning/src/ln/chanmon_update_fail_tests.rs
- lightning/src/ln/channel.rs
- lightning/src/ln/channelmanager.rs
- lightning/src/ln/monitor_tests.rs
- lightning/src/ln/shutdown_tests.rs
Additional comments: 4
lightning/src/ln/functional_tests.rs (4)
- 874-875: The use of
ClosureReason::CounterpartyInitiatedCooperativeClosureandClosureReason::LocallyInitiatedCooperativeClosureincheck_closed_event!calls correctly reflects the PR's objective to distinguish between closures initiated by the local node and those initiated by the remote counterparty. This change enhances the clarity of the test assertions by explicitly stating the expected closure reason, aligning with the modifications made to theClosureReasonenum.- 988-989: Similar to the previous comment, the explicit use of closure reason variants in
check_closed_event!calls here is a positive change. It ensures that the tests accurately reflect the new logic introduced in the PR for handling cooperative closures, thereby improving the test's readability and maintainability.- 1107-1117: The updates in this segment continue to apply the differentiated closure reasons across multiple channel closure scenarios. This consistent application of the new
ClosureReasonvariants across tests is crucial for validating the behavior of the system under various conditions of channel closure, ensuring that the implementation behaves as expected when initiated by either the local node or the remote counterparty.- 5622-5630: This segment demonstrates the application of the new closure reason variants in a scenario involving transaction mining and checking spendable outputs. The explicit distinction between
CounterpartyInitiatedCooperativeClosureandLocallyInitiatedCooperativeClosurein thecheck_closed_event!calls, even in the context of transaction finalization, underscores the thorough integration of the new logic throughout the test suite. This ensures that the behavior of the system is consistently tested against the new cooperative closure handling logic.
wpaulino
previously approved these changes
Feb 1, 2024
benthecarman
force-pushed
the
breakup-coop-close
branch
from
23c17f9 to
efcb5d9
Compare
|
fixed the typo |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- lightning/src/events/mod.rs (3 hunks)
- lightning/src/ln/chanmon_update_fail_tests.rs (2 hunks)
- lightning/src/ln/channel.rs (11 hunks)
- lightning/src/ln/channelmanager.rs (1 hunks)
- lightning/src/ln/functional_tests.rs (4 hunks)
- lightning/src/ln/monitor_tests.rs (1 hunks)
- lightning/src/ln/shutdown_tests.rs (13 hunks)
Files skipped from review as they are similar to previous changes (7)
- lightning/src/events/mod.rs
- lightning/src/ln/chanmon_update_fail_tests.rs
- lightning/src/ln/channel.rs
- lightning/src/ln/channelmanager.rs
- lightning/src/ln/functional_tests.rs
- lightning/src/ln/monitor_tests.rs
- lightning/src/ln/shutdown_tests.rs
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2863 +/- ##
==========================================
- Coverage 89.14% 89.14% -0.01%
==========================================
Files 116 116
Lines 93205 93290 +85
Branches 93205 93290 +85
==========================================
+ Hits 83089 83161 +72
- Misses 7583 7597 +14
+ Partials 2533 2532 -1 ☔ View full report in Codecov by Sentry. |
wpaulino
approved these changes
Feb 5, 2024
TheBlueMatt
approved these changes
Feb 5, 2024
| LegacyCooperativeClosure, | ||
| /// The channel was closed after negotiating a cooperative close and we've now broadcasted | ||
| /// the cooperative close transaction. This indicates that the shutdown was initiated by our | ||
| /// counterparty. |
There was a problem hiding this comment.
Suggested change
| /// counterparty. | |
| /// counterparty. | |
| /// | |
| /// In rare cases where we sent our peer a `shutdown`, crashed, then restarted this reason may be given even though we initiated the shutdown. |
k0k0ne
pushed a commit
to bitlightlabs/rust-lightning
that referenced
this pull request
Sep 30, 2024
v0.0.123 - May 08, 2024 - "BOLT12 Dust Sweeping" API Updates =========== * To reduce risk of force-closures and improve HTLC reliability the default dust exposure limit has been increased to `MaxDustHTLCExposure::FeeRateMultiplier(10_000)`. Users with existing channels might want to consider using `ChannelManager::update_channel_config` to apply the new default (lightningdevkit#3045). * `ChainMonitor::archive_fully_resolved_channel_monitors` is now provided to remove from memory `ChannelMonitor`s that have been fully resolved on-chain and are now not needed. It uses the new `Persist::archive_persisted_channel` to inform the storage layer that such a monitor should be archived (lightningdevkit#2964). * An `OutputSweeper` is now provided which will automatically sweep `SpendableOutputDescriptor`s, retrying until the sweep confirms (lightningdevkit#2825). * After initiating an outbound channel, a peer disconnection no longer results in immediate channel closure. Rather, if the peer is reconnected before the channel times out LDK will automatically retry opening it (lightningdevkit#2725). * `PaymentPurpose` now has separate variants for BOLT12 payments, which include fields from the `invoice_request` as well as the `OfferId` (lightningdevkit#2970). * `ChannelDetails` now includes a list of in-flight HTLCs (lightningdevkit#2442). * `Event::PaymentForwarded` now includes `skimmed_fee_msat` (lightningdevkit#2858). * The `hashbrown` dependency has been upgraded and the use of `ahash` as the no-std hash table hash function has been removed. As a consequence, LDK's `Hash{Map,Set}`s no longer feature several constructors when LDK is built with no-std; see the `util::hash_tables` module instead. On platforms that `getrandom` supports, setting the `possiblyrandom/getrandom` feature flag will ensure hash tables are resistant to HashDoS attacks, though the `possiblyrandom` crate should detect most common platforms (lightningdevkit#2810, lightningdevkit#2891). * `ChannelMonitor`-originated requests to the `ChannelSigner` can now fail and be retried using `ChannelMonitor::signer_unblocked` (lightningdevkit#2816). * `SpendableOutputDescriptor::to_psbt_input` now includes the `witness_script` where available as well as new proprietary data which can be used to re-derive some spending keys from the base key (lightningdevkit#2761, lightningdevkit#3004). * `OutPoint::to_channel_id` has been removed in favor of `ChannelId::v1_from_funding_outpoint` in preparation for v2 channels with a different `ChannelId` derivation scheme (lightningdevkit#2797). * `PeerManager::get_peer_node_ids` has been replaced with `list_peers` and `peer_by_node_id`, which provide more details (lightningdevkit#2905). * `Bolt11Invoice::get_payee_pub_key` is now provided (lightningdevkit#2909). * `Default[Message]Router` now take an `entropy_source` argument (lightningdevkit#2847). * `ClosureReason::HTLCsTimedOut` has been separated out from `ClosureReason::HolderForceClosed` as it is the most common case (lightningdevkit#2887). * `ClosureReason::CooperativeClosure` is now split into `{Counterparty,Locally}Initiated` variants (lightningdevkit#2863). * `Event::ChannelPending::channel_type` is now provided (lightningdevkit#2872). * `PaymentForwarded::{prev,next}_user_channel_id` are now provided (lightningdevkit#2924). * Channel init messages have been refactored towards V2 channels (lightningdevkit#2871). * `BumpTransactionEvent` now contains the channel and counterparty (lightningdevkit#2873). * `util::scid_utils` is now public, with some trivial utilities to examine short channel ids (lightningdevkit#2694). * `DirectedChannelInfo::{source,target}` are now public (lightningdevkit#2870). * Bounds in `lightning-background-processor` were simplified by using `AChannelManager` (lightningdevkit#2963). * The `Persist` impl for `KVStore` no longer requires `Sized`, allowing for the use of `dyn KVStore` as `Persist` (lightningdevkit#2883, lightningdevkit#2976). * `From<PaymentPreimage>` is now implemented for `PaymentHash` (lightningdevkit#2918). * `NodeId::from_slice` is now provided (lightningdevkit#2942). * `ChannelManager` deserialization may now fail with `DangerousValue` when LDK's persistence API was violated (lightningdevkit#2974). Bug Fixes ========= * Excess fees on counterparty commitment transactions are now included in the dust exposure calculation. This lines behavior up with some cases where transaction fees can be burnt, making them effectively dust exposure (lightningdevkit#3045). * `Future`s used as an `std::...::Future` could grow in size unbounded if it was never woken. For those not using async persistence and using the async `lightning-background-processor`, this could cause a memory leak in the `ChainMonitor` (lightningdevkit#2894). * Inbound channel requests that fail in `ChannelManager::accept_inbound_channel` would previously have stalled from the peer's perspective as no `error` message was sent (lightningdevkit#2953). * Blinded path construction has been tuned to select paths more likely to succeed, improving BOLT12 payment reliability (lightningdevkit#2911, lightningdevkit#2912). * After a reorg, `lightning-transaction-sync` could have failed to follow a transaction that LDK needed information about (lightningdevkit#2946). * `RecipientOnionFields`' `custom_tlvs` are now propagated to recipients when paying with blinded paths (lightningdevkit#2975). * `Event::ChannelClosed` is now properly generated and peers are properly notified for all channels that as a part of a batch channel open fail to be funded (lightningdevkit#3029). * In cases where user event processing is substantially delayed such that we complete multiple round-trips with our peers before a `PaymentSent` event is handled and then restart without persisting the `ChannelManager` after having persisted a `ChannelMonitor[Update]`, on startup we may have `Err`d trying to deserialize the `ChannelManager` (lightningdevkit#3021). * If a peer has relatively high latency, `PeerManager` may have failed to establish a connection (lightningdevkit#2993). * `ChannelUpdate` messages broadcasted for our own channel closures are now slightly more robust (lightningdevkit#2731). * Deserializing malformed BOLT11 invoices may have resulted in an integer overflow panic in debug builds (lightningdevkit#3032). * In exceedingly rare cases (no cases of this are known), LDK may have created an invalid serialization for a `ChannelManager` (lightningdevkit#2998). * Message processing latency handling BOLT12 payments has been reduced (lightningdevkit#2881). * Latency in processing `Event::SpendableOutputs` may be reduced (lightningdevkit#3033). Node Compatibility ================== * LDK's blinded paths were inconsistent with other implementations in several ways, which have been addressed (lightningdevkit#2856, lightningdevkit#2936, lightningdevkit#2945). * LDK's messaging blinded paths now support the latest features which some nodes may begin relying on soon (lightningdevkit#2961). * LDK's BOLT12 structs have been updated to support some last-minute changes to the spec (lightningdevkit#3017, lightningdevkit#3018). * CLN v24.02 requires the `gossip_queries` feature for all peers, however LDK by default does not set it for those not using a `P2PGossipSync` (e.g. those using RGS). This change was reverted in CLN v24.02.2 however for now LDK always sets the `gossip_queries` feature. This change is expected to be reverted in a future LDK release (lightningdevkit#2959). Security ======== 0.0.123 fixes a denial-of-service vulnerability which we believe to be reachable from untrusted input when parsing invalid BOLT11 invoices containing non-ASCII characters. * BOLT11 invoices with non-ASCII characters in the human-readable-part may cause an out-of-bounds read attempt leading to a panic (lightningdevkit#3054). Note that all BOLT11 invoices containing non-ASCII characters are invalid. In total, this release features 150 files changed, 19307 insertions, 6306 deletions in 360 commits since 0.0.121 from 17 authors, in alphabetical order: * Arik Sosman * Duncan Dean * Elias Rohrer * Evan Feenstra * Jeffrey Czyz * Keyue Bao * Matt Corallo * Orbital * Sergi Delgado Segura * Valentine Wallace * Willem Van Lint * Wilmer Paulino * benthecarman * jbesraa * olegkubrakov * optout * shaavan
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes the TODO listed in the reasons enum