Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Trip Circuit with Expiration #18402

Open
aesmonty opened this issue Nov 7, 2023 · 3 comments
Open

[Feature]: Trip Circuit with Expiration #18402

aesmonty opened this issue Nov 7, 2023 · 3 comments

Comments

@aesmonty
Copy link

aesmonty commented Nov 7, 2023

Summary

This improvement allows the Circuit module to specify a maximum amount of time that a circuit remains tripped for instead of the current implementation, which trips a circuit in perpetuity until the circuit is manually untripped.

It also included the extension of having authorization levels by timespan e.g., authority B can only trip IBC messages for a certain period of time such 6 hours.

Problem Definition

The current authorization levels (LEVEL_SUPER_ADMIN, LEVEL_ALL_MSGS, LEVEL_SOME_MSGS) introduce some granularity on the circuit breaker authorities. However, for certain actors such as security service providers or active validators, granting unlimited time tripping may be too powerful, raising concerns in the community.

Introducing expirations introduces another lever that will make the Circuit module more effective and decentralized. For most incident situations, a security provider could trip specific message types for a limited amount of time, which would give enough time for parties with "more authority," such as the core team or an expedited gov proposal, to review the situation and potentially trip the message for a longer time.

There is no disadvantage to adding this feature beyond the added complexity to the permissions system.

Proposed Feature

To handle the expired untripping of a circuit, the most efficient option would be to execute the business logic to untrip a circuit upon evaluation if the timespan has passed. The other option would be to handle circuit untripping in the BeforeBlock and EndBlock stages of IBC modules.

There are two approaches for handling the amount of time a circuit is tripped, detailed below.

1: No Expiration

This would extend the current implementation, allowing one to optionally specify the amount of time that the circuit should be tripped for. If no time is specified, then the circuit remains tripped in perpetuity.

2: Default Expiration Times

This approach would automatically assign a sensible amount of time to tripped circuits (i.e., 24 hours), requiring one to override the time explicitly.

Improved Permission Levels

An additional improvement would be to expand the permission levels to allow for specifying an upper bound to the timespan a circuit can be tripped for certain permission levels (ie, super admin can specify any timespan, other permissions capped at 24 hours).

Implementation Examples

To accommodate this feature, the DisabledList field will need to be changed to a [collections.Map] type or a new field introduced, which allows for mapping values to a key (message url).

Each key (message url) should map to an object that allows specifying expiration time and a list of addresses that are allowed to bypass the tripped circuit. An example of this is included below:

An implementation of the “deny by default” multi-address blocklist is given below, which would allow any signer included in the BypassSet to send a message regardless of whether or not the circuit is tripped.

type FilteredUrl struct {
   BypassSet collections.KeySet[string]
	 ExpiresAt i64
}

An updated AnteHandle method of the circuit breaker is listed below, with comments indicating the changed behavior.

func (cbd CircuitBreakerDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
  //  pseudocode, need to actually parse the transaction object to retrieve the signer
  signer := tx.GetSigners()
	// loop through all the messages and check if the message type is allowed
	for _, msg := range tx.GetMsgs() {

		// lookup message url in circuit breaker to see if it is tripped

		// if circuit is tripped, check to see if an expiration time is associated with the circuit

		// if there is an expiration time and it has passed untrip the circuit, and break out of loop

		// if there is an expiration time and it has not passed, or there is no expiration time
		// check to see if the signer is allowed to bypass the tripped circuit

		// circuit is tripped, signer can't bypass, return error
	}

	return next(ctx, tx, simulate)
}

Author: @bonedaddy Co-author: @aesmonty

@tac0turtle
Copy link
Member

the changes generally look good to me, We should avoid using begin block or end block if its not critical. We rely on those a bit too much for simple things

@aesmonty
Copy link
Author

aesmonty commented Nov 9, 2023

great to hear that @tac0turtle

let us know if you have any feedback or proposed changes. if not, we can go ahead and implement this feature

@alexanderbez
Copy link
Contributor

Proposal ACK. Granted, if an account is admin, I don't believe restrictions should apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

4 participants