ARC-0041: Update credits program to support delegation before validator bonding and native commission.

[evanmarshall]

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:

1. Delegators can delegate to any address (with the same 10,000 credit minimum & maximum of 100,000 delegators) even if that address is not in the committee
2. Validators only require a self bond of 100 credits to join the committee, given the total delegation for them is >= 10M credits.

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:

// 1. contains committee[r0] =>
//   a. contains delegated[r0]
//      - The set of delegated addresses is superset of active validators.
// 2. delegated[*] >= 10,000
//   a. contain bonded[*].microcredits >= 10,000
//      - Delegators can bond to any address with the 10,000 credit minimum
// 3. delegated[r0] < 10,000,000 =>
//   a. !contain committee[r0]
//      - If the delegated total (which includes the self bond) is less than 10,000,000 credits, it won't be in the committee
// 4. bond[r0].validator == r0 =>
//   a. contains committee[r0]
//     - Self bonded addresses are always active validators. Delegators **can** force validators to unbond by removing sufficient delegation
// 5. committee[r0] =>
//   a. bonded[r0].microcredits >= 100 credits
//      - Self bonded addresses are always active validators with at least 100 credits self bonded
//   b. delegated[r0] >= 10,000,000 credits
//      - The total delegated amount for an active validator is at least 10,000,000 credits
// 6. delegated[r0] == sum bond[*].microcredits by bond[].validator == r0
// 7. bond[r0].microcredits < 10,000 =>
//   a. bond[r0].validator == r0
//   b. contains committee[r0]
//   - A bond value < 10,000 implies self-bonded active validator.
// 8. bond[r0].validator != r0 =>
//   a. bond[r0].microcredits >= 10,000
// 9. count bonded[] - count committee[] == metadata[aleo1qgqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqanmpl0]
// 10. count committee[] == metadata[aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc]
// 11. 100,000 + count committee[] >= count bonded[] >= count delegated[] >= count committee[]
// 12. bond[*].microcredits > 100
// 13. delegated[*] > 10,000

API Changes

1. There is now a bond_validator function that should be used to add a validator to the committee instead of the bond_public that was used before. This separation function was added due to the change in interface to support specifying a commission.
2. The withdrawal address is now the address that needs to be used for unbond_public which applies to all stakers. There are 2 primary reasons for this change:
   • Security. Previously the validator hot key was used to unbond_public and claim_unbond_public while funds ended up at the address specified in the withdraw 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.
   • Programmability. We anticipate the need for compliance features, commission structures and validator pools. To broadly enable new use cases, we need a key that can sign certificates in BFT ie the validator key and an address that can have programmatically control staked funds. The validator key will always need to be a hot private key but the withdraw address can be a program so we're giving the withdraw address sufficient authority to manage staked funds.
3. claim_unbond_public now requires an address for claiming. claim_unbond_public previously used self.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

• Initial credits.aleo implementation
• snarkVM commission on rewards & staker checks
• snarkVM unit tests

Testing

1. Unit testing in snarkVM will be updated as part of this PR.
2. Before merging, we will run an isolated network with these changes.
3. Before merging, we will have comprehensive audits of the credits.aleo program and associated snarkVM changes.
4. After merging, we will first deploy on canary network before eventually running on testnet beta.

[iamalwaysuncomfortable]

Would it be possible to post a rendered markdown link to Arc-41 here?

[howardwu]

This PR is not ready for review. Converting to a draft.

[raychu86]

Just flagging that we had an alternative approach to ARC-38 commissions via a commission mapping (and not in the Committee struct) - #2354.

I realize the commissions have not been fully implemented in this PR, but I am unsure how you intend on updating the commission rates in the Committee.

[evanmarshall]

Just flagging that we had an alternative approach to ARC-38 commissions via a commission mapping (and not in the Committee struct) - #2354.

I realize the commissions have not been fully implemented in this PR, but I am unsure how you intend on updating the commission rates in the Committee.

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.

[evanmarshall]

Would it be possible to post a rendered markdown link to Arc-41 here?

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.

[kyle-u410]

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[].

[raychu86]

@d0cd

[evanmarshall]

Any reason not to use a u8 here or even a boolean?

[raychu86]

Initially metadata was supposed to be used for user accounts. We can change this to use a u8 if needed.

[d0cd]

Switching to a u8 would be fine.

[howardwu]

@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.

[ljedrz]

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.

[raychu86]

@evanmarshall Genesis blocks generated in - 8ba7dc5

[vicsn]

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:

• test_staking_rewards_when_staker_is_under_min_yields_no_reward - I assume this is because of the commission rate. Not sure if we should just remove assert_eq!(candidate_stake, stake); or explicitly recalculate the commission in the test.
• test_insufficient_public_fees
• test_max_committee_limit_with_bonds

[evanmarshall]

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:

• test_staking_rewards_when_staker_is_under_min_yields_no_reward - I assume this is because of the commission rate. Not sure if we should just remove assert_eq!(candidate_stake, stake); or explicitly recalculate the commission in the test.
• test_insufficient_public_fees
• test_max_committee_limit_with_bonds

These tests are fixed.

[vicsn]

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.

[apruden2008]

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.