-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
ARC-0041: Update credits program to support delegation before validator bonding and native commission. #2453
Conversation
TODO on updating unbond_public. Also includes attaching a commission to committee_state.
Would it be possible to post a rendered markdown link to Arc-41 here? |
This PR is not ready for review. Converting to a draft. |
Just flagging that we had an alternative approach to ARC-38 commissions via a commission mapping (and not in the I realize the |
The design for commissions here is that you can only set the commission as you enter the committee. Also, when trying to enter the committee, you cannot have any unbonding state. This effectively forces a validator to wait 360 blocks to leave and reenter the committee, to change their commission. The goal is to create a negative incentive for validators who may otherwise rapidly change their commission making it difficult for delegators to decide who to bond to. |
This doesn't really follow any ARC discussion thus far. I'm hoping with this Draft PR we can agree on a final design as there are many small decisions made here that may not be universally supported. Given the scope of the changes, I think it's more appropriate to file an ARC after we can agree upon a design to present to the community as the diff is quite large. |
// 8. bond[r0].validator != r0 => | ||
// a. bond[r0].microcredits >= 10,000 | ||
// 9. count bonded[] - count committee[] == metadata[aleo1qgqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqanmpl0] | ||
// 10. count committee[] == metadata[aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a good reason for using an address here instead of simple index?
The more I look at this metadata[0u32]
seems like a better alias for count committee[]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to use a u8
here or even a boolean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially metadata
was supposed to be used for user accounts. We can change this to use a u8 if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to a u8
would be fine.
}; | ||
|
||
// Extract the microcredits for the address from the delegated map. | ||
let microcredits = delegated_map.iter().find_map(|(delegated_key, delegated_value)| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljedrz can you assess this for performance implications? (use test_committee_and_delegated_maps_into_committee
for profiling)
We need a full committee with full delegated maps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That test takes roughly the same amount of time as before; however, due to the scope of these changes it would be best to run a profile in the IsoNet and compare it against one from a run with the current code if we'd like to make sure that there are no regressions.
…ated_maps_into_committee
…operations in bond_validator and bond_public
@evanmarshall Genesis blocks generated in - 8ba7dc5 |
Some tests are still failing. You can find fixes for some of them in this branch. I'll need input from @evanmarshall or @fulltimemike on:
|
These tests are fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI passes, review comments look incorporated, approving this so it can be tested in canary. If existing reviewers or auditors encounter issues, they can open new PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. We should work to add more test cases for this in the future, but in the meantime this looks good to merge in so we can start doing live testing on the canary net.
Great work @evanmarshall on pulling this together.
Summary
Currently, validators will need to have 10M credits in order to self.bond as a validator. The problem is that most or all of these validators won't actually have 10M credits. To bring Aleo in line with other ecosystems, we want to relax 2 existing constraints:
Although the changes are significant, the staking abstraction should closely follow what existed before.
Technical summary
The invariants of the
credits.aleo
program are now as follows:API Changes
bond_validator
function that should be used to add a validator to the committee instead of thebond_public
that was used before. This separation function was added due to the change in interface to support specifying a commission.withdrawal address
is now the address that needs to be used forunbond_public
which applies to all stakers. There are 2 primary reasons for this change:unbond_public
andclaim_unbond_public
while funds ended up at the address specified in thewithdraw
mapping. This meant that it was safe to lose exclusive access to the hot key but stakers still need to retain access to it to unlock their funds.claim_unbond_public
now requires an address for claiming.claim_unbond_public
previously usedself.caller
for the address. The result of this change is that anyone can pay the fee and run the transaction for someone to claim their unbonded balance. This removes an unnecessary restriction to further enhance security for operations (by reducing the reliance on a hot key and enabling separation of cold keys) and enable programmability.Progress
credits.aleo
implementationTesting