Skip to content

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Apr 28, 2022

Depends on #2959
See threshold-network/solidity-contracts#99

Introduced authorization decrease change period parameter to the beacon

This value protect against malicious operators who manipulate
their weight by overwriting authorization decrease request, and
lowering or increasing their eligible stake this way.

Authorization decrease change period is the time period before the authorization
decrease delay end, during which the authorization decrease request can be
overwritten.

When the request is overwritten, the authorization decrease delay is
reset.

For example, if authorizationDecraseChangePeriod is set to 4
days, authorizationDecreaseDelay is set to 14 days, and someone
requested authorization decrease, it means they can not
request another decrease for the first 10 days. After 10 days pass,
they can request again and overwrite the previous authorization
decrease request. The delay time will reset for them and they
will have to wait another 10 days to alter it and 14 days to
approve it.

If set to a value equal to `authorizationDecreaseDelay, it means
that authorization decrease request can be always overwritten.
If set to zero, it means authorization decrease request can not
be overwritten until the delay end, and one needs to wait for the entire
authorization decrease delay to approve their decrease or to alter it.

(1) authorization decrease requested timestamp
(2) from this moment authorization decrease request can be
    overwritten
(3) from this moment authorization decrease request can be
    approved, assuming it was NOT overwritten in (2)

  (1)                            (2)                        (3)
 --x------------------------------x--------------------------x---->
   |                               \________________________/
   |                             authorizationDecreaseChangePeriod
    \______________________________________________________/
                   authorizationDecreaseDelay

@pdyraga pdyraga requested a review from nkuba April 28, 2022 15:04
@pdyraga pdyraga self-assigned this Apr 28, 2022
This value protect against malicious operators who manipulate
their weight by overwriting authorization decrease request, and
lowering or increasing their eligible stake this way.

Authorization decrease change period is the time period before the authorization
decrease delay end, during which the authorization decrease request can be
overwritten.

When the request is overwritten, the authorization decrease delay is
reset.

For example, if `authorizationDecraseChangePeriod` is set to 4
days, `authorizationDecreaseDelay` is set to 14 days, and someone
requested authorization decrease, it means they can not
request another decrease for the first 10 days. After 10 days pass,
they can request again and overwrite the previous authorization
decrease request. The delay time will reset for them and they
will have to wait another 10 days to alter it and 14 days to
approve it.

If set to a value equal to `authorizationDecreaseDelay, it means
that authorization decrease request can be always overwritten.
If set to zero, it means authorization decrease request can not
be overwritten until the delay end, and one needs to wait for the entire
authorization decrease delay to approve their decrease or to alter it.

(1) authorization decrease requested timestamp
(2) from this moment authorization decrease request can be
    overwritten
(3) from this moment authorization decrease request can be
    approved, assuming it was NOT overwritten in (2)

  (1)                            (2)                        (3)
 --x------------------------------x--------------------------x---->
   |                               \________________________/
   |                             authorizationDecreaseChangePeriod
    \______________________________________________________/
                   authorizationDecreaseDelay
@pdyraga
Copy link
Member Author

pdyraga commented Apr 28, 2022

#2959 got merged so I am undrafting this one

Copy link
Member

@nkuba nkuba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left non-blocking comments that can be taken care of in a follow-up.

// will have to wait another 10 days to alter it and 14 days to
// approve it.
//
// This value protect against malicious operators who manipulate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// This value protect against malicious operators who manipulate
// This value protects against malicious operators who manipulate

decreasingBy,
decreasingAt
);
AuthorizationDecrease storage decreaseRequest = self.pendingDecreases[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AuthorizationDecrease storage decreaseRequest = self.pendingDecreases[
AuthorizationDecrease storage pendingDecrease = self.pendingDecreases[

).to.be.equal(deauthorizingSecond)
})

context("when delay passed", () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add one more test under "when change period is equal delay" to cover a case where the requestAuthorizationDecrease is called without any increaseTime?

})
})

context("when change period is zero", () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have two test cases under "when change period is zero":

  1. requestAuthorizationDecrease is called right away without any time increase
  2. requestAuthorizationDecrease is called after the delay period

@nkuba nkuba merged commit 8dec5d6 into main Apr 29, 2022
@nkuba nkuba deleted the authorization-stuff-2 branch April 29, 2022 11:32
@pdyraga pdyraga added this to the solidity/v2.0.0 milestone Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants