From d4348969693c436b4d44a479da974139bcbe3318 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 9 Jun 2023 14:41:24 -0700 Subject: [PATCH 01/14] all of ADR is filled out except design portion --- docs/docs/adrs/adr-008-throttle-retries.md | 68 ++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 docs/docs/adrs/adr-008-throttle-retries.md diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md new file mode 100644 index 0000000000..6f21dd1002 --- /dev/null +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -0,0 +1,68 @@ +--- +sidebar_position: 7 +title: Throttle with retries +--- + +# ADR {008}: {Throttle with retries} + +## Changelog + +* {6/9/23}: Initial draft + +## Status + +> A decision may be "proposed" if it hasn't been agreed upon yet, or "accepted" once it is agreed upon. If a later ADR changes or reverses a decision, it may be marked as "deprecated" or "superseded" with a reference to its replacement. + +Proposed + +## Context + +For context on why the throttling mechanism exists, see [ADR 002](./adr-002-throttle.md). + +Note the terms slash throttling and jail throttling are synonymous, since in replicated security a `SlashPacket` simply jails a validator for downtime infractions. + +Currently the throttling mechanism is designed so that provider logic (slash meter, etc.) dictates how many slash packets can be handled over time. Throttled slash packets are persisted on the provider, leading to multiple possible issues. Namely: + +* If slash or vsc matured packets are actually throttled/queued on the provider, state can grow and potentially lead to a DoS attack. We have short term solutions around this, but overall they come with their own weaknesses. See [#594](https://github.com/cosmos/interchain-security/issues/594). +* If a jailing attack described in [ADR 002](adr-002-throttle.md) were actually to be carried out with the current throttling design, we'd likely have to halt the provider, and perform an emergency upgrade and/or migration to clear the queues of slash packets that were deemed to be malicious. Alternatively, validators would just have to _tough it out_ and wait for the queues to clear, during which all/most validators would be jailed. Right after being jailed, vals would have to unjail themselves promptly to ensure safety. The synchronous coordination required to maintain safety in such a scenario is not ideal. + +So what's the solution? We can improve the throttling mechanism to instead queue/persist relevant data on each consumer, and have consumers retry slash requests as needed. + +## Decision + +> This section explains all of the details of the proposed solution, including implementation details. +It should also describe affects / corollary items that may need to be changed as a part of this. +If the proposed change will be large, please also indicate a way to do the change to maximize ease of review. +(e.g. the optimal split of things to do between separate PR's) + +## Consequences + +> This section describes the consequences, after applying the decision. All consequences should be summarized here, not just the "positive" ones. + +* Consumers will now have to manage their own queues, and retry logic. +* Consumers still aren't trustless, but the provider is now less susceptible to mismanaged or malicious consumers. +* Recovering from the "jailing attack" is more elegant. +* Some issues like [#1001](https://github.com/cosmos/interchain-security/issues/1001) will now be handled implicitly by the improved throttling mechanism. +* Slash and vsc matured packets can be handled immediately once recv by the provider if the slash meter allows. +* In general, we reduce the amount of computation that happens in the provider end-blocker. + +### Positive + +* We no longer have to reason about a "global queue" and a "chain specific queue", and keeping those all in-sync. Now slash and vsc matured packet queuing is handled on each consumer individually. +* Due to the above, the throttling protocol becomes less complex overall. +* We no longer have to worry about throttle related DoS attack on the provider, since no queuing exists on the provider. + +### Negative + +* Increased number of IBC packets being relayed anytime throttling logic is triggered. +* Consumer complexity increases, since consumers now have manage queuing themselves, and implement packet retry logic. + +### Neutral + +* Core throttling logic on the provider remains unchanged, ie. slash meter, replenishment cycles, etc. + +## References + +* [EPIC](https://github.com/cosmos/interchain-security/issues/713) +* [ADR 002](./adr-002-throttle.md) +* [#594](https://github.com/cosmos/interchain-security/issues/594) \ No newline at end of file From 4090734229e18949032f61c40baf69bbb9b8d28c Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 9 Jun 2023 15:55:25 -0700 Subject: [PATCH 02/14] design --- docs/docs/adrs/adr-008-throttle-retries.md | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 6f21dd1002..6b9d190403 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -35,6 +35,34 @@ It should also describe affects / corollary items that may need to be changed as If the proposed change will be large, please also indicate a way to do the change to maximize ease of review. (e.g. the optimal split of things to do between separate PR's) +### Provider changes + +The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets received by the provider will always be handled immediately upon being received by the provider. + +Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send the packets in the correct order, and the ordered IBC channel will ensure that the packets are received in the correct order on the provider. + +### Consumer changes + +Note the consumer already queues up both slash and vsc matured packets via `AppendPendingPacket`. Those packets are dequeued every endblock in `SendPackets` and sent to the provider. + +Instead, we will now introduce the following logic on endblock: + +* Slash packets will always be sent to the provider once they're at the head of the queue. However, once sent, the consumer will not send any trailing vsc matured packets from the queue until the provider responds with an ack that the slash packet has been handled (ie. val was jailed). That is, slash packets block the sending of trailing vsc matured packets in the consumer queue. +* If two slash packets are at the head of the queue, the consumer will send the first slash packet, and then wait for a success ack from the provider before sending the second slash packet. This seems like it'd simplify implementation. +* VSC matured packets at the head of the queue (ie. NOT trailing a slash packet) can be sent immediately, and do not block any other packets in the queue, since the provider always handles them immediately. + +Note IBC doesn't implement an event subscription model between chains, so the consumer will have to retry the sending of slash packets over some period of time. This can be achieved with an on chain consumer param. The suggested param value would probably be 1/2 of the provider's `SlashMeterReplenishmentPeriod`, although it doesn't matter too much as long as the param value is sane. + +Note to prevent weird edge case behavior, a retry would not be attempted until either a success ack or failure ack has been recv from the provider. + +With the behavior described, we maintain very similar behavior to the current throttling mechanism regarding the timing that slash and vsc matured packets are handled on the provider. Obviously the queueing and blocking logic is moved, and the two chains do have to send more messages between one another. + +In the nominal case (no or minimal slash packets being sent), the vsc matured packets will not be delayed, and hence unbonding will not be delayed. + +### Splitting of PRs + +We could split this feature into two PRs, one affecting the consumer and one affecting the provider, along with a third PR which could setup a clever way to upgrade the provider in multiple steps, ensuring that queued slash packets at upgrade time are handled properly. + ## Consequences > This section describes the consequences, after applying the decision. All consequences should be summarized here, not just the "positive" ones. From 0a1628109675528882567189b1c95e99e0c5676c Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 9 Jun 2023 15:58:00 -0700 Subject: [PATCH 03/14] Update adr-008-throttle-retries.md --- docs/docs/adrs/adr-008-throttle-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 6b9d190403..8052a7c0e4 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -37,7 +37,7 @@ If the proposed change will be large, please also indicate a way to do the chang ### Provider changes -The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets received by the provider will always be handled immediately upon being received by the provider. +The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send the packets in the correct order, and the ordered IBC channel will ensure that the packets are received in the correct order on the provider. From acccfe9ededd77aef7fff71c3cd4597a1283876e Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 9 Jun 2023 16:02:51 -0700 Subject: [PATCH 04/14] Update adr-008-throttle-retries.md --- docs/docs/adrs/adr-008-throttle-retries.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 8052a7c0e4..ce90f75ab1 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -39,7 +39,9 @@ If the proposed change will be large, please also indicate a way to do the chang The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. -Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send the packets in the correct order, and the ordered IBC channel will ensure that the packets are received in the correct order on the provider. +Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send packets in the correct order and block packet sending as needed. Then the ordered IBC channel will ensure that the packets are received in the correct order on the provider. + +The provider's main responsibility regarding throttling will now be to determine if a recv slash packet can be handled via slash meter etc., and appropriately ack to the sending consumer. ### Consumer changes From e8297911cbb483be0647b7b32ab153c508f74e4e Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 9 Jun 2023 16:06:17 -0700 Subject: [PATCH 05/14] Update adr-008-throttle-retries.md --- docs/docs/adrs/adr-008-throttle-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index ce90f75ab1..0eefcd056c 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -53,7 +53,7 @@ Instead, we will now introduce the following logic on endblock: * If two slash packets are at the head of the queue, the consumer will send the first slash packet, and then wait for a success ack from the provider before sending the second slash packet. This seems like it'd simplify implementation. * VSC matured packets at the head of the queue (ie. NOT trailing a slash packet) can be sent immediately, and do not block any other packets in the queue, since the provider always handles them immediately. -Note IBC doesn't implement an event subscription model between chains, so the consumer will have to retry the sending of slash packets over some period of time. This can be achieved with an on chain consumer param. The suggested param value would probably be 1/2 of the provider's `SlashMeterReplenishmentPeriod`, although it doesn't matter too much as long as the param value is sane. +To prevent the provider from having to keep track of what slash packets have been rejected, the consumer will have to retry the sending of slash packets over some period of time. This can be achieved with an on-chain consumer param. The suggested param value would probably be 1/2 of the provider's `SlashMeterReplenishmentPeriod`, although it doesn't matter too much as long as the param value is sane. Note to prevent weird edge case behavior, a retry would not be attempted until either a success ack or failure ack has been recv from the provider. From c939e1579c464729a428e22fab4b8d280ef2eff7 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 10:57:24 -0700 Subject: [PATCH 06/14] Apply suggestions from code review Co-authored-by: Marius Poke --- docs/docs/adrs/adr-008-throttle-retries.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 0eefcd056c..11750ab328 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -59,7 +59,7 @@ Note to prevent weird edge case behavior, a retry would not be attempted until e With the behavior described, we maintain very similar behavior to the current throttling mechanism regarding the timing that slash and vsc matured packets are handled on the provider. Obviously the queueing and blocking logic is moved, and the two chains do have to send more messages between one another. -In the nominal case (no or minimal slash packets being sent), the vsc matured packets will not be delayed, and hence unbonding will not be delayed. +In the normal case, when no or a few slash packets are being sent, the VSCMaturedPacketswill not be delayed, and hence unbonding will not be delayed. ### Splitting of PRs @@ -93,6 +93,6 @@ We could split this feature into two PRs, one affecting the consumer and one aff ## References -* [EPIC](https://github.com/cosmos/interchain-security/issues/713) -* [ADR 002](./adr-002-throttle.md) +* [EPIC](https://github.com/cosmos/interchain-security/issues/713) tracking the changes proposed by this ADR +* [ADR 002: Jail Throttling](./adr-002-throttle.md) * [#594](https://github.com/cosmos/interchain-security/issues/594) \ No newline at end of file From 5f20a634993791e5f6567ecd1e74bfe448125336 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:55:13 -0700 Subject: [PATCH 07/14] nit formatting --- docs/docs/adrs/adr-008-throttle-retries.md | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 11750ab328..ce6986621a 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -3,16 +3,14 @@ sidebar_position: 7 title: Throttle with retries --- -# ADR {008}: {Throttle with retries} +## ADR 008: Throttle with retries ## Changelog -* {6/9/23}: Initial draft +* 6/9/23: Initial draft ## Status -> A decision may be "proposed" if it hasn't been agreed upon yet, or "accepted" once it is agreed upon. If a later ADR changes or reverses a decision, it may be marked as "deprecated" or "superseded" with a reference to its replacement. - Proposed ## Context @@ -30,11 +28,6 @@ So what's the solution? We can improve the throttling mechanism to instead queue ## Decision -> This section explains all of the details of the proposed solution, including implementation details. -It should also describe affects / corollary items that may need to be changed as a part of this. -If the proposed change will be large, please also indicate a way to do the change to maximize ease of review. -(e.g. the optimal split of things to do between separate PR's) - ### Provider changes The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. @@ -63,12 +56,10 @@ In the normal case, when no or a few slash packets are being sent, the VSCMature ### Splitting of PRs -We could split this feature into two PRs, one affecting the consumer and one affecting the provider, along with a third PR which could setup a clever way to upgrade the provider in multiple steps, ensuring that queued slash packets at upgrade time are handled properly. +We could split this feature into two PRs, one affecting the consumer and one affecting the provider, along with a third PR which could setup a clever way to upgrade the provider in multiple steps, ensuring that queued slash packets at upgrade time are handled properly. ## Consequences -> This section describes the consequences, after applying the decision. All consequences should be summarized here, not just the "positive" ones. - * Consumers will now have to manage their own queues, and retry logic. * Consumers still aren't trustless, but the provider is now less susceptible to mismanaged or malicious consumers. * Recovering from the "jailing attack" is more elegant. From 87e343e323e5ece79e052e39f062cdd8a422b529 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:57:01 -0700 Subject: [PATCH 08/14] describe consumer changes first --- docs/docs/adrs/adr-008-throttle-retries.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index ce6986621a..156a720fd5 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -28,14 +28,6 @@ So what's the solution? We can improve the throttling mechanism to instead queue ## Decision -### Provider changes - -The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. - -Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send packets in the correct order and block packet sending as needed. Then the ordered IBC channel will ensure that the packets are received in the correct order on the provider. - -The provider's main responsibility regarding throttling will now be to determine if a recv slash packet can be handled via slash meter etc., and appropriately ack to the sending consumer. - ### Consumer changes Note the consumer already queues up both slash and vsc matured packets via `AppendPendingPacket`. Those packets are dequeued every endblock in `SendPackets` and sent to the provider. @@ -54,6 +46,14 @@ With the behavior described, we maintain very similar behavior to the current th In the normal case, when no or a few slash packets are being sent, the VSCMaturedPacketswill not be delayed, and hence unbonding will not be delayed. +### Provider changes + +The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. + +Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send packets in the correct order and block packet sending as needed. Then the ordered IBC channel will ensure that the packets are received in the correct order on the provider. + +The provider's main responsibility regarding throttling will now be to determine if a recv slash packet can be handled via slash meter etc., and appropriately ack to the sending consumer. + ### Splitting of PRs We could split this feature into two PRs, one affecting the consumer and one affecting the provider, along with a third PR which could setup a clever way to upgrade the provider in multiple steps, ensuring that queued slash packets at upgrade time are handled properly. From 66bc340ed5f6b6f4b4cd3bef07d283e1ee07cf98 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:58:35 -0700 Subject: [PATCH 09/14] add comment on rareness of throttling being triggered --- docs/docs/adrs/adr-008-throttle-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 156a720fd5..e5e558d302 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -42,7 +42,7 @@ To prevent the provider from having to keep track of what slash packets have bee Note to prevent weird edge case behavior, a retry would not be attempted until either a success ack or failure ack has been recv from the provider. -With the behavior described, we maintain very similar behavior to the current throttling mechanism regarding the timing that slash and vsc matured packets are handled on the provider. Obviously the queueing and blocking logic is moved, and the two chains do have to send more messages between one another. +With the behavior described, we maintain very similar behavior to the current throttling mechanism regarding the timing that slash and vsc matured packets are handled on the provider. Obviously the queueing and blocking logic is moved, and the two chains would have to send more messages between one another (only in the case the throttling mechanism is triggered). In the normal case, when no or a few slash packets are being sent, the VSCMaturedPacketswill not be delayed, and hence unbonding will not be delayed. From 0aa3b5302329bdc2fa138c3350ec96b247272ef0 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:00:21 -0700 Subject: [PATCH 10/14] split out paragraph --- docs/docs/adrs/adr-008-throttle-retries.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index e5e558d302..f74701f6ce 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -48,7 +48,11 @@ In the normal case, when no or a few slash packets are being sent, the VSCMature ### Provider changes -The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). VSCMatured packets will always be handled immediately upon being received by the provider. +The main change needed for the provider is the removal of queuing logic for slash and vsc matured packets upon being received. + +Instead, the provider will consult the slash meter to determine if a slash packet can be handled immediately. If not, the provider will return an ack message to the consumer communicating that the slash packet could not be handled, and needs to be sent again in the future (retried). + +VSCMatured packets will always be handled immediately upon being received by the provider. Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send packets in the correct order and block packet sending as needed. Then the ordered IBC channel will ensure that the packets are received in the correct order on the provider. From 701112f6888bd97cfb40366cb68ab8477c18042a Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:28:26 -0700 Subject: [PATCH 11/14] hopefully better explanation --- docs/docs/adrs/adr-008-throttle-retries.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index f74701f6ce..531c791b07 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -54,10 +54,22 @@ Instead, the provider will consult the slash meter to determine if a slash packe VSCMatured packets will always be handled immediately upon being received by the provider. -Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via its queuing. Now the onus will be on the consumer to send packets in the correct order and block packet sending as needed. Then the ordered IBC channel will ensure that the packets are received in the correct order on the provider. +Note [spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#consumer-initiated-slashing). Specifically the section on _VSC Maturity and Slashing Order_. Previously the onus was on the provider to maintain this property via queuing packets and handling them FIFO. + +Now this property will be maintained by the consumer sending packets in the correct order, and blocking the sending of VSCMatured packets as needed. Then, the ordered IBC channel will ensure that Slash/VSCMatured packets are received in the correct order on the provider. The provider's main responsibility regarding throttling will now be to determine if a recv slash packet can be handled via slash meter etc., and appropriately ack to the sending consumer. +### Why the provider can handle VSCMatured packets immediately + +First we answer, what does a VSCMatured packet communicate to the provider? A VSCMatured packet communicates that a VSC has been applied to a consumer long enough that infractions committed on the consumer could have been submitted. + +If the consumer is following the queuing/blocking protocol described. No bad behavior occurs, `VSC Maturity and Slashing Order` property is maintained. + +If a consumer sends VSCMatured packets too leniently: The consumer is malicious and sending duplicate vsc matured packets, or sending the packets sooner than the ccv protocol specifies. In this scenario, the provider needs to handle vsc matured packets immediately to prevent DOS, state bloat, or other issues. The only possible negative outcome is that the malicious consumer may not be able to jail a validator who should have been jailed. The malicious behavior only creates a negative outcome for the chain that is being malicious. + +If a consumer blocks the sending of VSCMatured packets: The consumer is malicious and blocking vsc matured packets that should have been sent. This will block unbonding only up until the VSC timeout period has elapsed. At that time, the consumer is removed. Again the malicious behavior only creates a negative outcome for the chain that is being malicious. + ### Splitting of PRs We could split this feature into two PRs, one affecting the consumer and one affecting the provider, along with a third PR which could setup a clever way to upgrade the provider in multiple steps, ensuring that queued slash packets at upgrade time are handled properly. From fbf05a21c933313f107dc94ed9e389508d9781d3 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:34:13 -0700 Subject: [PATCH 12/14] Update adr-008-throttle-retries.md --- docs/docs/adrs/adr-008-throttle-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index 531c791b07..d7e892c906 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -44,7 +44,7 @@ Note to prevent weird edge case behavior, a retry would not be attempted until e With the behavior described, we maintain very similar behavior to the current throttling mechanism regarding the timing that slash and vsc matured packets are handled on the provider. Obviously the queueing and blocking logic is moved, and the two chains would have to send more messages between one another (only in the case the throttling mechanism is triggered). -In the normal case, when no or a few slash packets are being sent, the VSCMaturedPacketswill not be delayed, and hence unbonding will not be delayed. +In the normal case, when no or a few slash packets are being sent, the VSCMaturedPackets will not be delayed, and hence unbonding will not be delayed. ### Provider changes From dc8d655ef5545d1cbadd53318b31950b6096d7a7 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 14 Jun 2023 09:35:28 -0700 Subject: [PATCH 13/14] accepted --- docs/docs/adrs/adr-008-throttle-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-008-throttle-retries.md b/docs/docs/adrs/adr-008-throttle-retries.md index d7e892c906..a8f0d250ce 100644 --- a/docs/docs/adrs/adr-008-throttle-retries.md +++ b/docs/docs/adrs/adr-008-throttle-retries.md @@ -11,7 +11,7 @@ title: Throttle with retries ## Status -Proposed +Accepted ## Context From c79ffc99e864520f3dcc67e4cc1786e503ff2f13 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu, 15 Jun 2023 09:53:21 -0700 Subject: [PATCH 14/14] TOC entry --- docs/docs/adrs/intro.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/docs/adrs/intro.md b/docs/docs/adrs/intro.md index aa6b7781c9..baf99aec4c 100644 --- a/docs/docs/adrs/intro.md +++ b/docs/docs/adrs/intro.md @@ -34,4 +34,5 @@ To suggest an ADR, please make use of the [ADR template](./adr-template.md) prov | ------ | ----------- | ------ | | [001](./adr-001-key-assignment.md) | Consumer chain key assignment | Accepted, Implemented | | [002](./adr-002-throttle.md) | Jail Throttling | Accepted, Implemented | -| [003](./adr-003-equivocation-gov-proposal.md) | Equivocation governance proposal | Accepted, Implemented +| [003](./adr-003-equivocation-gov-proposal.md) | Equivocation governance proposal | Accepted, Implemented | +| [008](./adr-008-throttle-retries.md) | Throttle with retries | Accepted, In-progress |