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

Enable DAO Proposal to execute Arbitray Tx #19

Closed
wants to merge 27 commits into from

Conversation

DerTiedemann
Copy link

@DerTiedemann DerTiedemann commented Aug 25, 2022

Goals

Allow more or less (anything the implements a specific tx type) to be executed as a ConcludeProposalTx that will only be executed when the vote has passed. Also allow the Implementing Tx to verify the ProposalConfiguration to make sure it can reject parameters that are invalid. E.g. not high enough threshold or invalid durration of Proposal.

Non-Goals

Allow for multiple outcomes in the same proposal

Implementation

Newly/Adjusted Tx:

  • SubmitDaoProposalTx -> Creates and typechecks a proposal for a wrapped proposedTX (Currently only AddValidatorTx)
  • AddDaoVoteTx -> Any currently active validator that has been a validator before the vote comenced can issue approval with this tx
  • ConcludeDaoProposalTx -> is Issued automatically by the node and executes the proposed tx if the proposal was accepted

Caveats

The proposedTx is checked at submit time, but the state can change while the proposal is active. If the proposalTx becomes invalid while the Proposal is invalid the proposal concludes as normal and ignores the failure. That might not be desireable but the easiest way of handeling it atm.

The AddValidator proposal will not be able to submited outside of a ConcludeProposalTx

@@ -306,6 +324,30 @@ func (m *blockBuilder) Shutdown() {
m.vm.ctx.Lock.Lock()
}

// getStakerToReward return the staker txID to remove from the primary network
Copy link
Member

Choose a reason for hiding this comment

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

this documentation seems to have been copied from another method

@@ -215,6 +215,34 @@ type Client interface {
GetBlock(ctx context.Context, blockID ids.ID, options ...rpc.Option) ([]byte, error)
// GetConfiguration returns genesis information of the primary network
GetConfiguration(ctx context.Context) (*GetConfigurationReply, error)
// // AddDaoProposal adds a new Proposal created by this nodeId.
Copy link
Member

Choose a reason for hiding this comment

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

hey @DerTiedemann , these were the commented out methods I was referring to..

return errStartTimeBeforeEndTime
}

if d.Thresh > 0 {
Copy link
Member

@knikos knikos Aug 31, 2022

Choose a reason for hiding this comment

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

that should be rather
d.Thresh <= 0

case daoProposal.Voted(tx.NodeID):
return nil, nil, fmt.Errorf("node %s has already voted on proposal: %s", tx.NodeID.String(), tx.ProposalID.String())
case daoProposal.DaoProposalTx().StartTime().After(validator.AddValidatorTx().StartTime()):
return nil, nil, fmt.Errorf("node %s only became a validator after the proposal started and is thus not eligible to participate", tx.NodeID.String())
Copy link
Member

@knikos knikos Sep 1, 2022

Choose a reason for hiding this comment

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

That should rather be:
daoProposal.DaoProposalTx().StartTime().Before(validator.AddValidatorTx().StartTime()):

return state == ProposalStateConcluded
}

// StartTime is the time that this validator will enter the validator set
Copy link
Member

Choose a reason for hiding this comment

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

this documentation is obviously copied from validator.go and does not describe its real purpose

@DerTiedemann
Copy link
Author

Damm, i just noticed, that there is am implicit requirement for the Ins of both the Submit and WrappedTx to be disjunct. Which the current stake function does not care about... sadge

@DerTiedemann DerTiedemann marked this pull request as draft September 2, 2022 00:49
@DerTiedemann
Copy link
Author

I converted i back to a draft as i found some bugs that are game breaking, ill need to discuss with @evlekht how to proceed.
Main Issue:

  • At tx creation time you will need to reference two set of ins that are disjunct for the Submit & Wrapped Tx, the current implementaiton of spend does not do this and the service does also not take this into account which will lead to the Tx no being accepted.

Ill need to see if there is a way to sidestep this issue.

@DerTiedemann
Copy link
Author

superceded by #269

@evlekht evlekht deleted the jax/TECH-168/dao-rewrite branch July 4, 2024 15:23
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