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

[Uptime Incentives]: Add authorized uptime validation for new NoLock gauges #7367

Closed
Tracked by #7366
AlpinYukseloglu opened this issue Jan 25, 2024 · 0 comments · Fixed by #7376
Closed
Tracked by #7366

[Uptime Incentives]: Add authorized uptime validation for new NoLock gauges #7367

AlpinYukseloglu opened this issue Jan 25, 2024 · 0 comments · Fixed by #7376

Comments

@AlpinYukseloglu
Copy link
Contributor

AlpinYukseloglu commented Jan 25, 2024

Background

Since we will be using gauge duration in place of uptime, we need to add the critical guardrail of disallowing future gauges to be created with invalid or unauthorized uptimes. We will address existing “invalid” gauges in a future issue by making them fall back to the default uptime of 1ns. The reasons for this are explained in #7369.

Suggested Design

A clever way to implement this pretty concisely would be to add the following logic to the existing gauge duration validation logic:
main...alpo/enable-uptime-gauges#diff-d57570c2b30c7fe37108bd7847d1419ad3f0240d7dd28d37e0aecff0730236c8R131-R139

This essentially updates the current validation logic to check gauge durations against authorized uptimes instead of lockable durations if the gauge is for a CL pool. For the sake of simplicity, the gaugeType abstraction in the linked code should be replaced with a NoLock type check, as this is how we currently identify CL gauges.

We also need to add two additional checks to protect against edge cases, the first of which would be directly exploitable if not included:

Edit: it seems the first check is done later in the function prior to any gauge creation logic, so this check does not seem to be necessary. It should be ensured that it is tested anyway.

  1. If the gauge type is NoLock, fetch the pool and verify that it is a CL pool as done here:
    pool, err := k.GetPoolFromGaugeId(ctx, gauge.Id, gauge.DistributeTo.Duration)
    if err != nil {
    return nil, err
    }
    poolType := pool.GetType()
    if poolType != poolmanagertypes.Concentrated {
    return nil, fmt.Errorf("pool type %s is not supported for no lock distribution", poolType)
    }
    Otherwise, one can create a balancer gauge with NoLock type to bypass lockup duration validation.
  2. Make an exception to validation logic if it’s coming from the Pool Incentives module account, as these are internal gauges for CL and will be using the gov param for distributions anyway. If this is not added, the post pool creation hook (that makes internal gauges) for CL will fail, potentially causing serious issues (no more CL pool creation at best, chain halt vector at worst)

Testing Strategy

The following tests should be added to existing gauge creation tests (or a new set if necessary):

  • Create gauge with authorized uptime (happy path)
  • Attempt to create gauge with unauthorized uptime (error)
  • Attempt to create with unauthorized (error) -> authorize the uptime and try again -> works
  • Authorize an uptime -> create with authorized uptime (works) -> unauthorize the uptime and try again -> errors
  • Attempt to create a NoLock gauge on a Balancer pool (error)
  • Ensure epoch duration / 1 day is unauthorized and attempt to create CL pool. Should work. Ensure internal gauge was created for the pool using k.pik.GetInternalGaugeIDForPool(ctx, poolID)

Acceptance Criteria

  • New validation logic is added and thoroughly tested
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant