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

Dao - Validator Voting #5

Closed
wants to merge 13 commits into from
Closed

Dao - Validator Voting #5

wants to merge 13 commits into from

Conversation

c4t-ag
Copy link

@c4t-ag c4t-ag commented May 24, 2022

Validator DAO Voting

  • Everyone can place a Proposal (API: AddDaoProposal)
  • Vote threshold is (#Validators +1 ) / 2
  • Only Validators are allowed to vote for a proposal (AddDaoVote)
    (Must be called from one of the addresses the stake was provided)
  • GetDaoProposal retrieves one proposal at given proposalID
    In case the proposal time has ended, only the state (voted / unknown) is returned

AddDaoProposal includes a start timestamp (must be > now + 20 secs) and an end timestamp.
You can pass 0 as startTime (now) and a number of seconds (end) and the API will base this on timestamp now.

Default settings (genesis) will be documented in https://docs.camino.foundation

There is a check for duplicate ProposalType / ProposalData combinations -> they will be rejected.

Only one vote per Validator is possible, even your stake is made out of n addresses / UTXO's

Currently all Proposals and Votes live in the chain state and can be queried. There are plans that we discard old proposals (+ Votes) after a longer time to save reources.
Deleting directly at the time the proposal ends would be not a good idea because there need to be time to handle actions based on the result (like addValidator).

When a proposal period ends, for accepted proposals only the proposalID is stored in state to allow later verification.
Non-accepted votes will be dicarded from state, an can only be reproduced using history transactions.

@c4t-ag c4t-ag requested a review from Noctunus May 24, 2022 17:03
@peak3d peak3d force-pushed the dao branch 2 times, most recently from 45f07ee to dc86a77 Compare May 24, 2022 20:06
@evlekht evlekht self-requested a review July 6, 2022 15:34
@@ -254,6 +248,17 @@ func (vm *VM) stake(
})
}

// Handle fees after stake to reduce UTXO generation
Copy link
Member

Choose a reason for hiding this comment

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

I'd move this change to dedicated PR, so we can separatly test and merge it without consideration of other changes


proposalCache := service.vm.internalState.DaoProposalChainState()
proposal, err := proposalCache.GetActiveProposal(args.DaoProposalID)
if err == nil {
Copy link
Member

Choose a reason for hiding this comment

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

I'd change this and code below to err != nil with error response on not found proposal. It will be more clear this way, imho

@@ -2060,63 +2322,63 @@ type GetStakeReply struct {
// Returns:
Copy link
Member

Choose a reason for hiding this comment

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

I'd replace index int with enum

@@ -53,6 +56,8 @@ type TxFeeConfig struct {
CreateSubnetTxFee uint64 `json:"createSubnetTxFee"`
// Transaction fee for create blockchain transactions
CreateBlockchainTxFee uint64 `json:"createBlockchainTxFee"`
// Fee that must be burned by every create staker transaction
AddStakerTxFee uint64 `json:"addStakerTxFee"`
Copy link
Member

Choose a reason for hiding this comment

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

Must be supported wth changes in camino-node

@@ -42,6 +43,8 @@ type StakingConfig struct {
MaxStakeDuration time.Duration `json:"maxStakeDuration"`
// RewardConfig is the config for the reward function.
RewardConfig reward.Config `json:"rewardConfig"`
// DaoConfig is the config for the dao function.
DaoConfig dao.Config `json:"daoConfig"`
Copy link
Member

Choose a reason for hiding this comment

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

Must be supported wth changes in camino-node

@@ -217,6 +218,35 @@ func (fx *Fx) VerifyCredentials(tx Tx, in *Input, cred *Credential, out *OutputO
return nil
}

// GetPublicKeys recovers public keys from tx and Signature and stores it
// in the map provided.
func (fx *Fx) GetPublicKeys(iTx, iCreds, iMap interface{}) error {
Copy link
Member

Choose a reason for hiding this comment

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

Why args as interface{} ?


// Computes the id of this proposal
func (d *Proposal) computeID() (ids.ID, error) {
toSerialize := [5]uint64{d.ProposalType, d.Wght, d.Start, d.End, uint64(d.Thresh)}
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use proposal tx ID? To prevent proposals of similar type exist at the same time?
As I understand, 2 proposals could have equal ID only if they'll start and end on the same time with the same weight, tresh. and type. But they could still contain different data, which will mean that they'r different.
Can you, please, explain idea behind this ID in general

// Our Dao Proposal
DaoProposal dao.Proposal `serialize:"true" json:"daoProposal"`

// Where to send locked tokens when done voting
Copy link
Member

Choose a reason for hiding this comment

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

Its locked outs on tx execution, no on end of voting, right?

copy(outs, tx.Outs)
copy(outs[len(tx.Outs):], tx.Locks)

if vm.bootstrapped.GetValue() {
Copy link
Member

Choose a reason for hiding this comment

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

Stupid question for my education: why do we need node bootstrapped here? Not doubting, just wanna understand better

errVotingTooLong = errors.New("voting period is too long")
errAlreadyValidator = errors.New("node is already a validator")

_ UnsignedProposalTx = &UnsignedDaoProposalTx{}
Copy link
Member

Choose a reason for hiding this comment

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

Probably missing TimedTx check

stakedAmt, outs, err := service.getStakeHelper(tx, addrs)
if err != nil {
return err
for i := 0; i < 3; i++ {
Copy link
Member

Choose a reason for hiding this comment

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

This whole thing with for-loop and if-s inside looks messy and hard to understand. I'd rewrite it

@evlekht
Copy link
Member

evlekht commented Sep 29, 2023

This PR is now outdated (see #269), so I'm closing it.

@evlekht evlekht closed this Sep 29, 2023
@evlekht evlekht deleted the dao branch July 4, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants