validator remove, and fix unauthorized leave (#378)

* authorized leave

* remove validator

cmd/kwil-admin/validator.go

@@ -15,13 +15,15 @@ import (
 // kwil-admin validators join
 // kwil-admin validators approve
 // kwil-admin validators leave
+// kwil-admin validators remove
 
 type ValidatorsCmd struct {
 	List       *ValListCmd       `arg:"subcommand:list"`
 	JoinStatus *ValJoinStatusCmd `arg:"subcommand:join-status"`
 	Join       *ValJoinCmd       `arg:"subcommand:join"`
 	Approve    *ValApproveCmd    `arg:"subcommand:approve"`
 	Leave      *ValLeaveCmd      `arg:"subcommand:leave"`
+	Remove     *ValRemoveCmd     `arg:"subcommand:remove"`
 
 	RPCServer    string `arg:"-s,--rpcserver" default:"127.0.0.1:50051" help:"RPC server address"`
 	ChainID      string `arg:"-c,--chain" default:"" help:"The Kwil network's chain ID to use for transactions"`
@@ -157,6 +159,28 @@ func (vac *ValApproveCmd) run(ctx context.Context, a *args) error {
 	return display.Print(display.RespTxHash(txHash), err, a.Vals.OutputFormat)
 }
 
+type ValRemoveCmd struct {
+	Target HexArg `arg:"positional,required" help:"Public key of the validator to propose to remove."`
+	valSignedCmd
+}
+
+var _ runner = (*ValRemoveCmd)(nil)
+
+func (vac *ValRemoveCmd) run(ctx context.Context, a *args) error {
+	var txHash []byte
+	err := func() error {
+		rpcAddr, chainID := a.Vals.RPCServer, a.Vals.ChainID
+		clt, err := vac.client(ctx, rpcAddr, chainID)
+		if err != nil {
+			return err
+		}
+		txHash, err = clt.RemoveValidator(ctx, vac.Target)
+		return err
+	}()
+
+	return display.Print(display.RespTxHash(txHash), err, a.Vals.OutputFormat)
+}
+
 type ValLeaveCmd struct {
 	valSignedCmd
 }

core/client/client.go

@@ -335,6 +335,24 @@ func (c *Client) ApproveValidator(ctx context.Context, joiner []byte, opts ...Tx
 	return hash, nil
 }
 
+// RemoveValidator makes a transaction proposing to remove a validator. This is
+// only useful if the Client's signing key is a current validator key.
+func (c *Client) RemoveValidator(ctx context.Context, target []byte, opts ...TxOpt) ([]byte, error) {
+	_, err := crypto.Ed25519PublicKeyFromBytes(target)
+	if err != nil {
+		return nil, fmt.Errorf("invalid candidate validator public key: %w", err)
+	}
+	payload := &transactions.ValidatorRemove{
+		Validator: target,
+	}
+	tx, err := c.newTx(ctx, payload, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	return c.transportClient.Broadcast(ctx, tx)
+}
+
 func (c *Client) ValidatorJoin(ctx context.Context) ([]byte, error) {
 	const power = 1
 	return c.validatorUpdate(ctx, power)
@@ -345,17 +363,12 @@ func (c *Client) ValidatorLeave(ctx context.Context) ([]byte, error) {
 }
 
 func (c *Client) validatorUpdate(ctx context.Context, power int64, opts ...TxOpt) ([]byte, error) {
-	pubKey := c.Signer.PublicKey()
-
 	var payload transactions.Payload
 	if power <= 0 {
-		payload = &transactions.ValidatorLeave{
-			Validator: pubKey,
-		}
+		payload = &transactions.ValidatorLeave{}
 	} else {
 		payload = &transactions.ValidatorJoin{
-			Candidate: pubKey,
-			Power:     uint64(power),
+			Power: uint64(power),
 		}
 	}
 

core/types/transactions/payloads.go

@@ -21,6 +21,7 @@ func (p PayloadType) Valid() bool {
 		PayloadTypeCallAction,
 		PayloadTypeValidatorJoin,
 		PayloadTypeValidatorApprove,
+		PayloadTypeValidatorRemove,
 		PayloadTypeValidatorLeave:
 		return true
 	default:
@@ -35,6 +36,7 @@ const (
 	PayloadTypeCallAction       PayloadType = "call_action"
 	PayloadTypeValidatorJoin    PayloadType = "validator_join"
 	PayloadTypeValidatorLeave   PayloadType = "validator_leave"
+	PayloadTypeValidatorRemove  PayloadType = "validator_remove"
 	PayloadTypeValidatorApprove PayloadType = "validator_approve"
 )
 
@@ -263,8 +265,7 @@ func (a *ActionCall) Type() PayloadType {
 }
 
 type ValidatorJoin struct {
-	Candidate []byte
-	Power     uint64
+	Power uint64
 }
 
 func (v *ValidatorJoin) Type() PayloadType {
@@ -301,10 +302,27 @@ func (v *ValidatorApprove) MarshalBinary() ([]byte, error) {
 	return serialize.Encode(v)
 }
 
-type ValidatorLeave struct {
+type ValidatorRemove struct {
 	Validator []byte
 }
 
+func (v *ValidatorRemove) Type() PayloadType {
+	return PayloadTypeValidatorRemove
+}
+
+var _ encoding.BinaryUnmarshaler = (*ValidatorRemove)(nil)
+var _ encoding.BinaryMarshaler = (*ValidatorRemove)(nil)
+
+func (v *ValidatorRemove) UnmarshalBinary(b []byte) error {
+	return serialize.DecodeInto(b, v)
+}
+
+func (v *ValidatorRemove) MarshalBinary() ([]byte, error) {
+	return serialize.Encode(v)
+}
+
+type ValidatorLeave struct{}
+
 func (v *ValidatorLeave) Type() PayloadType {
 	return PayloadTypeValidatorLeave
 }

core/types/transactions/payloads_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/kwilteam/kwil-db/core/types/transactions"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -124,13 +125,16 @@ func Test_Types(t *testing.T) {
 		{
 			name: "validator_join",
 			obj: &transactions.ValidatorJoin{
-				Candidate: []byte("asdfadsf"),
-				Power:     1,
+				Power: 1,
 			},
 		},
 		{
 			name: "validator_leave",
-			obj: &transactions.ValidatorLeave{
+			obj:  &transactions.ValidatorLeave{},
+		},
+		{
+			name: "validator_remove",
+			obj: &transactions.ValidatorRemove{
 				Validator: []byte("asdfadsf"),
 			},
 		},
@@ -159,6 +163,8 @@ func Test_Types(t *testing.T) {
 				obj = &transactions.ValidatorJoin{}
 			case *transactions.ValidatorLeave:
 				obj = &transactions.ValidatorLeave{}
+			case *transactions.ValidatorRemove:
+				obj = &transactions.ValidatorRemove{}
 			default:
 				t.Fatal("unknown type")
 			}

core/types/types.go

@@ -40,6 +40,13 @@ type Validator struct {
 	Power  int64
 }
 
+// ValidatorRemoveProposal is a proposal from an existing validator (remover) to
+// remove a validator (the target) from the validator set.
+type ValidatorRemoveProposal struct {
+	Target  []byte
+	Remover []byte
+}
+
 func (v *Validator) String() string {
 	return fmt.Sprintf("{pubkey = %x, power = %d}", v.PubKey, v.Power)
 }

internal/abci/abci.go

@@ -410,6 +410,7 @@ func (a *AbciApp) DeliverTx(req abciTypes.RequestDeliverTx) abciTypes.ResponseDe
 		}
 
 		gasUsed = res.GasUsed
+
 	case transactions.PayloadTypeValidatorJoin:
 		var join transactions.ValidatorJoin
 		err = join.UnmarshalBinary(tx.Body.Payload)
@@ -419,34 +420,29 @@ func (a *AbciApp) DeliverTx(req abciTypes.RequestDeliverTx) abciTypes.ResponseDe
 		}
 
 		logger.Debug("join validator",
-			zap.String("pubkey", hex.EncodeToString(join.Candidate)),
+			zap.String("pubkey", hex.EncodeToString(tx.Sender)),
 			zap.Int64("power", int64(join.Power)))
 
 		var res *modVal.ExecutionResponse
-		res, err = a.validators.Join(ctx, join.Candidate, int64(join.Power), tx)
+		res, err = a.validators.Join(ctx, int64(join.Power), tx)
 		if err != nil {
 			txCode = codeUnknownError
 			break
 		}
-		// Concept:
-		// if res.Error != "" {
-		// 	err = errors.New(res.Error)
-		// 	gasUsed = res.Fee.Int64()
-		// 	break
-		// }
 
 		events = []abciTypes.Event{
 			{
 				Type: "validator_join",
 				Attributes: []abciTypes.EventAttribute{
 					{Key: "Result", Value: "Success", Index: true},
-					{Key: "ValidatorPubKey", Value: hex.EncodeToString(join.Candidate), Index: true},
+					{Key: "ValidatorPubKey", Value: hex.EncodeToString(tx.Sender), Index: true},
 					{Key: "ValidatorPower", Value: fmt.Sprintf("%d", join.Power), Index: true},
 				},
 			},
 		}
 
 		gasUsed = res.GasUsed
+
 	case transactions.PayloadTypeValidatorLeave:
 		var leave transactions.ValidatorLeave
 		err = leave.UnmarshalBinary(tx.Body.Payload)
@@ -455,10 +451,10 @@ func (a *AbciApp) DeliverTx(req abciTypes.RequestDeliverTx) abciTypes.ResponseDe
 			break
 		}
 
-		logger.Debug("leave validator", zap.String("pubkey", hex.EncodeToString(leave.Validator)))
+		logger.Debug("leave validator", zap.String("pubkey", hex.EncodeToString(tx.Sender)))
 
 		var res *modVal.ExecutionResponse
-		res, err = a.validators.Leave(ctx, leave.Validator, tx)
+		res, err = a.validators.Leave(ctx, tx)
 		if err != nil {
 			txCode = codeUnknownError
 			break
@@ -469,13 +465,14 @@ func (a *AbciApp) DeliverTx(req abciTypes.RequestDeliverTx) abciTypes.ResponseDe
 				Type: "validator_leave",
 				Attributes: []abciTypes.EventAttribute{
 					{Key: "Result", Value: "Success", Index: true},
-					{Key: "ValidatorPubKey", Value: hex.EncodeToString(leave.Validator), Index: true},
+					{Key: "ValidatorPubKey", Value: hex.EncodeToString(tx.Sender), Index: true},
 					{Key: "ValidatorPower", Value: "0", Index: true},
 				},
 			},
 		}
 
 		gasUsed = res.GasUsed
+
 	case transactions.PayloadTypeValidatorApprove:
 		var approve transactions.ValidatorApprove
 		err = approve.UnmarshalBinary(tx.Body.Payload)
@@ -505,6 +502,37 @@ func (a *AbciApp) DeliverTx(req abciTypes.RequestDeliverTx) abciTypes.ResponseDe
 		}
 
 		gasUsed = res.GasUsed
+
+	case transactions.PayloadTypeValidatorRemove:
+		var remove transactions.ValidatorRemove
+		err = remove.UnmarshalBinary(tx.Body.Payload)
+		if err != nil {
+			txCode = codeEncodingError
+			break
+		}
+
+		logger.Debug("remove validator", zap.String("pubkey", hex.EncodeToString(remove.Validator)))
+
+		var res *modVal.ExecutionResponse
+		res, err = a.validators.Remove(ctx, remove.Validator, tx)
+		if err != nil {
+			txCode = codeUnknownError
+			break
+		}
+
+		events = []abciTypes.Event{
+			{
+				Type: "validator_remove",
+				Attributes: []abciTypes.EventAttribute{
+					{Key: "Result", Value: "Success", Index: true},
+					{Key: "TargetPubKey", Value: hex.EncodeToString(remove.Validator), Index: true},
+					{Key: "RemoverPubKey", Value: hex.EncodeToString(tx.Sender), Index: true},
+				},
+			},
+		}
+
+		gasUsed = res.GasUsed
+
 	default:
 		err = fmt.Errorf("unknown payload type: %s", tx.Body.PayloadType.String())
 	}
@@ -529,7 +557,11 @@ func (a *AbciApp) EndBlock(e abciTypes.RequestEndBlock) abciTypes.ResponseEndBlo
 	logger := a.log.With(zap.String("stage", "ABCI EndBlock"), zap.Int("height", int(e.Height)))
 	logger.Debug("", zap.Int64("height", e.Height))
 
-	a.valUpdates = a.validators.Finalize(context.Background())
+	var err error
+	a.valUpdates, err = a.validators.Finalize(context.Background())
+	if err != nil {
+		panic(fmt.Sprintf("failed to finalize validator updates: %v", err))
+	}
 
 	valUpdates := make([]abciTypes.ValidatorUpdate, len(a.valUpdates))
 	for i, up := range a.valUpdates {

internal/abci/interfaces.go

@@ -47,18 +47,21 @@ type ValidatorModule interface {
 	Punish(ctx context.Context, validator []byte, power int64) error
 
 	// Join creates a join request for a prospective validator.
-	Join(ctx context.Context, joiner []byte, power int64, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
+	Join(ctx context.Context, power int64, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
 	// Leave processes a leave request for a validator.
-	Leave(ctx context.Context, joiner []byte, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
+	Leave(ctx context.Context, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
 	// Approve records an approval transaction from a current validator. The
 	// approver is the tx Sender.
 	Approve(ctx context.Context, joiner []byte, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
+	// Remove removes a validator from the validator set, if the sender is a
+	// current validator.
+	Remove(ctx context.Context, validator []byte, tx *transactions.Transaction) (*modVal.ExecutionResponse, error)
 
 	// Finalize is used at the end of block processing to retrieve the validator
 	// updates to be provided to the consensus client for the next block. This
 	// is not idempotent. The modules working list of updates is reset until
 	// subsequent join/approves are processed for the next block.
-	Finalize(ctx context.Context) []*validators.Validator // end of block processing requires providing list of updates to the node's consensus client
+	Finalize(ctx context.Context) ([]*validators.Validator, error) // end of block processing requires providing list of updates to the node's consensus client
 
 	// Updates block height stored by the validator manager. Called in the abci Commit
 	UpdateBlockHeight(ctx context.Context, blockHeight int64)
@@ -71,6 +74,9 @@ type ValidatorModule interface {
 
 	// PriceLeave returns the price of a leave transaction.
 	PriceLeave(ctx context.Context) (*big.Int, error)
+
+	// PriceRemove returns the price of a remove transaction.
+	PriceRemove(ctx context.Context) (*big.Int, error)
 }
 
 // AtomicCommitter is an interface for a struct that implements atomic commits across multiple stores

internal/modules/validators/interfaces.go

@@ -19,10 +19,12 @@ type ValidatorMgr interface {
 	Join(ctx context.Context, joiner []byte, power int64) error
 	Leave(ctx context.Context, joiner []byte) error
 	Approve(ctx context.Context, joiner, approver []byte) error
-	Finalize(ctx context.Context) []*validators.Validator // end of block processing requires providing list of updates to the node's consensus client
+	Remove(ctx context.Context, target, validator []byte) error
+	Finalize(ctx context.Context) ([]*validators.Validator, error) // end of block processing requires providing list of updates to the node's consensus client
 	UpdateBlockHeight(blockHeight int64)
 
 	PriceJoin(ctx context.Context) (*big.Int, error)
 	PriceApprove(ctx context.Context) (*big.Int, error)
 	PriceLeave(ctx context.Context) (*big.Int, error)
+	PriceRemove(ctx context.Context) (*big.Int, error)
 }

internal/modules/validators/status.go

@@ -33,7 +33,7 @@ func (vm *ValidatorModule) Punish(ctx context.Context, validator []byte, newPowe
 // not idempotent. The modules working list of updates is reset until subsequent
 // join/approves are processed for the next block. end of block processing
 // requires providing list of updates to the node's consensus client
-func (vm *ValidatorModule) Finalize(ctx context.Context) []*validators.Validator {
+func (vm *ValidatorModule) Finalize(ctx context.Context) ([]*validators.Validator, error) {
 	return vm.mgr.Finalize(ctx)
 }