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

Lien on Staked BLD #3277

Closed
rowgraus opened this issue Jun 9, 2021 · 16 comments
Closed

Lien on Staked BLD #3277

rowgraus opened this issue Jun 9, 2021 · 16 comments
Assignees
Labels
BLD-Boost Issues relating to the BLD Boost contract and UI Core Economy OBSOLETE in favor of INTER-protocol duplicate Epic
Milestone

Comments

@rowgraus
Copy link

rowgraus commented Jun 9, 2021

No description provided.

@rowgraus rowgraus added Core Economy OBSOLETE in favor of INTER-protocol BLD-Boost Issues relating to the BLD Boost contract and UI labels Jun 9, 2021
@rowgraus rowgraus added this to the Mainnet: Phase 1 - Treasury Launch milestone Jun 9, 2021
@rowgraus
Copy link
Author

rowgraus commented Jun 9, 2021

Design:

  • At the point that the user is trying to undelegate, Go module asks Javascript if that's ok

Notes:

  • Representation of lien has to be in Javascript regardless - per Michael much easier to only have this in one place in JS

  • Kate: separate token would indicate that the tokens are locked-up. Contract needs to know how much BLD has been delegated

  • Go produces information around the amount of staked assets

  • Per Kate: should the ERTP token

  • A contract represents a lien against tokens

@JimLarson JimLarson self-assigned this Jun 14, 2021
@michaelfig
Copy link
Member

michaelfig commented Jun 14, 2021

Notes for @JimLarson:

Two possible directions for Golang:

a. create a separate x/vlien module and change x/staking to use it.
b. make minimal adjustments to x/staking, and call it from x/vstaking.
c. use a general staking derivatives design.

I think we prefer b, and eventually c.

@JimLarson
Copy link
Contributor

Goals

  1. Allow staked BLD to be used as collateral so that it cannot be finish unbonding until all encumberances have been resolved. We expect blocked unbondings to be relatively rare.

  2. Communicate changes in staking delegation so that JS is aware of the staking distribution.

The tracking of the encumbrance is done at the Javascript level, while the dynamics of staking, slashing, and unbonding is done in the Cosmos x/staking module and friends. We don't want to replicate the logic of the standard modules, and would like to keep modifications of them to a minimum. We also want to play nice with the ongoing staking epoching changes.

We should provide an intuitive user experience for the situation of blocked unbonding, ideally without needing modification to current clients of staking state.

Design

  1. A modification to x/staking to add a hook to the end-block processing of Unbonding Delegations to possibly block some amount from being removed from the queue.
  2. A new module, x/vstaking, that serves the above hook and listen for staking change events and reflects them over the bridge to JS.
  3. JS-side logic to send/receive bridge messages to enforce liens.

A blocked unbonding will be seen as perpetually in the "unbonding" state to observers, which is an adequate user experience.

x/staking Modifications

There is an existing facility for adding StakingHooks, following a pattern used in several other cosmos-sdk modules, but it's a bad fit for several reasons:

  • All other hook methods return void and do not affect staking state.
  • A staking hook object implements all methods, so all staking hooks would gain permission to block unbonding, which is too much authority.
  • All existing staking hooks would have to be modified to implement the new method.
  • Staking can be configured with multiple hooks, so we'd need to invent behavior to combine results, though this was never the intent.

The SDK does have precedent for passing a callback function to iterator methods for keepers. While this is a different use case, it seems closer and more appropriate.

So we'll add the following to the staking keeper:

type Reserver func(ctx sdk.Context, u UnbondingDelegation) sdk.Int
UnbondingReserver(cb Reserver) bool

The UnboindingReserver() call will set a new reserver callback and return whether the reserver was previous unset. The staking module will call the reserver, if set, before removing and processing an entry from the UnbondingDelegation queue. The reserver returns the amount to reserve on the queue, so a reserver which allows everything to pass can just unconditionally return zero.

x/vstaking Module

This module accepts a staking keeper in its constructor and registers an unbonding reserver with it. The reserver will simply send the call parameters over the bridge and receive a reply indicating the amount to reserve.

If liens are generally resolved before delegations are unbonded, then this results in a single JS call per unbonding. However, once an unbonding is blocked, even partially, and left on the UnbondingDelegations queue, it will be processed at every EndBlock until the lien is resolved. Each blocked unbonding will result in a separate JS call. If this leads to unacceptable overheads, we could change the x/vstaking module to keep state and the protocol with JS would modify the state only on lien state changes.

The x/vstaking module also listens to the following event types from the x/staking module and reflects them as messages over the bridge so that the JS side can mainain a picture of the staking state.

  • create_validator
  • edit_validator
  • delegate
  • redelegate, complete_redelegation
  • unbond, complete_unbonding

(Implementation option: could also implement StakingHooks to get these signals.)

New Bridge Messages

  • Undelegation Request: should some of the tokens be reserved?
    • type: VSTAKING_UNDELEGATE_RESERVE
    • delegator: string, bech32 address
    • validator: string, bech32 address
    • denom: string
    • amount: string, amount to be released
    • block height?, timestamp?
    • nonce for matching reply?
  • Undelegation Response
    • amount: string, amount to withhold, "0" to release everything
  • Validator creation
    • type: VSTAKING_CREATE_VALIDATOR
    • validator: string, bech32 address
  • Delegation event
    • type: VSTAKING_DELEGATE
    • delegator: string, bech32 address
    • validator: string, bech32 address
    • denom: string
    • amount: string, amount delegated
  • Redelegation event
    • type: VSTAKING_REDELEGATE
    • delegator: string, bech32 address
    • src_validator: string, bech32 address
    • dst_validator: string, bech32 address
    • denom: string
    • amount: string, amount delegated
  • Unbonding event
    • type: VSTAKING_UNBOND
    • delegator: string, bech32 address
    • validator: string, bech32 address
    • denom: string
    • amount: string, amount unbonded

Alternative: Upon any change to a delegator's state with a given validator, simply give the new amounts, e.g.

  • Staking update
    • type: VSTAKING_UPDATE
    • delegator: string, bech32 address
    • updates: list of
      • validator: string, bech32 address
      • denom: string
      • amount: string, amount delegated

Question: should we give the denom everywhere to be explicit, or just assume that it's common knowledge?

@JimLarson
Copy link
Contributor

Requesting input on the above from @katelynsills and @Chris-Hibbert

@Chris-Hibbert
Copy link
Contributor

Chris-Hibbert commented Jun 29, 2021

As discussed previously, I think the javascript side wants to know when validators drop out (unbonding, slashing) as well as when they're added. Several of the methods proposed in that doc are for tracking the rewards, so they're unnecessary here. (endBlock, slashing, unjail).

Do you need entries for bonding? (i.e. a validator has existed for a while, and its delegation share grew enough that it's now in the active set, or someone else was slashed, and it was the next biggest, so it has now been added to the top-N list.)

I like staking update.

Undelegation Request: should some of the tokens be reserved?

Yes.

block height?, timestamp?

I went back and forth about whether the recipient should be assumed to have an independent authority on the time. I ended up decided the timestamp (and block height) didn't have to be included in nearly every call.

nonce for matching reply?

Whatever go's style is on this will work.

@michaelfig
Copy link
Member

  • nonce for matching reply?

The Golang<->JS bridge is synchronous from the perspective of Golang and returns a string value to Go. JS can do any amount of async work before resolving the return result, and that is JSON-encoded and returned to Golang. So, no need to match replies at the application layer, that's already done for you.

@michaelfig
Copy link
Member

Question: should we give the denom everywhere to be explicit, or just assume that it's common knowledge?

Please always include denom, and as per our lesson learned in #3435, be sure to sort the updates.

@dckc
Copy link
Member

dckc commented Oct 5, 2021

I asked @JimLarson to clarify a discussion in the neighborhood of this issue, and he came up with a story that helps me at least some:

Bob has 5000 BLD tokens, half delegated to ABCCo, half to XYZCo.. Bob gets an attestation for these 5000 staked tokens - either for voting or a LoC. ABCCo withdraws from the validation business and 2500 of Bob's tokens become unbonded. Bob extends his attestation: that's fine - the unbonded tokens are still encumbered by a lien and can't be transferred out. Now XYZCo gets slashed and 500 of Bob's staked tokens are taken away. Bob tries to extend his attestation, but this fails, as Bob only has 4500 tokens left.

@dtribble asked @rowgraus to capture it.

@JimLarson @rowgraus if that story matches the scope of this issue pretty well, let's move it up to the description. If that story is only part of the scope of this issue, it's probably still putting it there.
If there's a more relevant issue, let's put it there.

@JimLarson
Copy link
Contributor

Clarity on the need for attestation extension: a delegator needs to be able to vote on multiple issues with overlapping voting periods, hence overlapping attestations. Extensions make it explicit that we're reusing an existing lien rather than creating a new one.

@JimLarson
Copy link
Contributor

Notes from a discussion today with @rowgraus @Chris-Hibbert and @dtribble.

Extensions

To extend an attestation, we'd like to ensure that there are still enough bonded tokens to cover the attestation. The current plan is to assume attestations can be extended unless notified otherwise. Once an attestation is created, the number of bonded tokens can decrease through voluntary unbonding, involuntary unbonding, and slashing.

Previous plan for lien JS <--> Go interface for slashing notification was for Go to make an upcall to JS to notify it of addresses which had been affected by slashing. Unfortunately, it is difficult to do this accurately without extensive modification of existing Cosmos modules. The mechanism for an unrelated module to know about slashing is event notification, and the slashing event message is missing the infraction time. This means that for unbonding delegations and redelegations we can't accurately tell whether the account was slashed or not.

Another difficulty is that the list of all addresses affected by a slashing event could result in a message that's prohibitively large. It would be better to have a more incremental overhead to avoid this sort of spike, even if the average overhead is higher.

Note that we have no plans to send a notification for voluntary or involuntary unbonding.

Note also that there might be enough bonded tokens left in an account after unbonding or slashing to cover for existing liens. What we'd really like to know is if the currently bonded tokens are "under water" with respect to the existing liens.

The overhead of making a GetAccountState() call for each extension is thought to be acceptable. This would solve all the above problems.

Proxying

On the assumptions that our validators will be more attuned to current events on the blockchain than the average delegator, we want validators to be able to vote the delegated tokens by default, but also allow delegators to explicitly vote if they desire.

The simplest way to handle the bookkeeping is to know the current delegations of a delegator when an attestation is created or extended. Any vote using that attestation deducts from the strength of the votes of the delegated validator.

In the meeting we discussed having a new call to return the amount and start time of all delegations, which would be called at extension time. The attestation contract would record a recent history of slashing events (validator and timestamp), so the delegation information could also be used to know which accounts might be affected by slashing.

However, given the considerations of the first section, it seems better to include the delegation information in the return from GetAccountState(), call it on both attestation creation or extension, and then there would be no need for slashing notification.

@dckc
Copy link
Member

dckc commented Oct 6, 2021

I asked @JimLarson to catch me up on why anyone would want / need to extend an attestation. He provided the following story:

Suppose proposal A is up for vote
JL is going to vote all his 1000 tokens Yes on proposal A
the system [?] has to verify that I have those tokens staked (aka bonded), i.e. that my position is at-stake and helping to secure the system

can't transfer tokens out to prevent
bad situation: JL and MF each vote, transfer tokens to each other, and vote again

mechanism: attestation; something is "observed to be the case." includes a duration (as well as account address, amount). only given provided there's a (cosmos-level) lien.

prop B comes up for vote
JL want to vote in that too.
If JL tries to get a separate attestation for his 1000 tokens, he'll lose.
so he can extend.

@Chris-Hibbert
Copy link
Contributor

I'd phrase this slightly differently. We have attestations that represent a right to vote that can be presented to a vote counter. The attestations have a good-until date that reflects the duration of the lock-up period they're subject to. Since the attestations expire, voters need to be able to get replacements with a longer duration when the expiration time approaches. The vote counters need to be able to recognize the new attestation as representing the same right to vote as the old one.

So the basic mechanism in the attestation contract is that you present your right to vote, and either present evidence of a longer lock-up, or ask the contract to confirm it for you with a call down to the Cosmos level. We currently represent the attestation with a re-usable ERTP payment, but it could as easily be a single use payment that you replace when you need a new one by talking to the attestation contract. In either case, the attestation contract can issue new payments that reflects a voters current lock-up.

@dckc
Copy link
Member

dckc commented Oct 27, 2021

@JimLarson notes this overlaps with #3991 ; perhaps overtaken.

@rowgraus rowgraus added the MN-1 label Jan 19, 2022
@Tartuffo Tartuffo changed the title Design: Lien on Staked BLD Lien on Staked BLD Jan 26, 2022
@Tartuffo
Copy link
Contributor

@JimLarson Can this be closed? If not, is it for MN-1?

@JimLarson
Copy link
Contributor

Prefer to keep open until we've done end-to-end use. But since I have no known work items left to do, we can close if you'd prefer.

@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@Tartuffo Tartuffo removed this from the Mainnet: Phase 1 - RUN Protocol milestone Feb 8, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@Tartuffo Tartuffo modified the milestones: Mainnet 1, RUN Protocol RC0 Apr 5, 2022
@Tartuffo Tartuffo added the Epic label Jun 15, 2022
@dckc
Copy link
Member

dckc commented Jul 7, 2022

dup of #3788

@dckc dckc closed this as completed Jul 7, 2022
@dckc dckc added the duplicate label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLD-Boost Issues relating to the BLD Boost contract and UI Core Economy OBSOLETE in favor of INTER-protocol duplicate Epic
Projects
None yet
Development

No branches or pull requests

6 participants