Add the ability to mark an account as Non-Participating. (algorand#261)

* Add ability to become nonpart to keyreg txns, and add corresponding edit to keyreg test * Add check in WellFormed re: key reg txns Add test for this behavior * Remove an unrelated comment that is wrong. * Add marknonparticipating command to goal. Untested WIP * Add test for go-nonparticipating to onlineStatusChange_test.go (formerly known as goOnlineGoOffline_test.go) Add MakeUnsignedBecomeNonparticipatingTx to libgoal Use libgoal.MakeUnsignedBecomeNonparticipatingTx in goal * Add a new consensus version. Add a flag to consensus version indicating support of become-nonparticipating transactions. Check this flag in keyreg.go:apply. * Check in `WellFormed` and `apply` for the case where nonpart-transactions are not supported, but a transaction is marking an account nonparticipatory * Add a unit test for new error behavior * Fix test regression. Refactor WellFormed slightly. * test increasing confirmation deadline since more transactions are being issued * Only do the "nonparticipation transaction" part of the test on those networks supporting nonparticipation transactions * Test out (WIP) a very high timeout on this test, to confirm it's not the timeout * Fix the bug in the onlineStatusChange_test * Address review comment: goal marknonparticipating description * Address review comment: use ConsensusVFuture * Address review comment: newline in imports * `make sanity`, per PR guidelines. * Skip become-nonpart tests if they are not supported * Add a version of onlineStatusChange_test that tests future consensus protocols. Also, add requisite template for doing this
EOSTribe · Sep 25, 2019 · e48332f · e48332f
1 parent b819395
commit e48332f
Show file tree

Hide file tree

Showing 13 changed files with 349 additions and 53 deletions.
diff --git a/cmd/goal/account.go b/cmd/goal/account.go
@@ -40,26 +40,26 @@ import (
 )
 
 var (
-	accountAddress     string
-	walletName         string
-	defaultAccountName string
-	defaultAccount     bool
-	unencryptedWallet  bool
-	online             bool
-	accountName        string
-	transactionFee     uint64
-	onlineFirstRound   uint64
-	onlineValidRounds  uint64
-	onlineTxFile       string
-	roundFirstValid    uint64
-	roundLastValid     uint64
-	keyDilution        uint64
-	threshold          uint8
-	partKeyOutDir      string
-	partKeyFile        string
-	partKeyDeleteInput bool
-	importDefault      bool
-	mnemonic           string
+	accountAddress          string
+	walletName              string
+	defaultAccountName      string
+	defaultAccount          bool
+	unencryptedWallet       bool
+	online                  bool
+	accountName             string
+	transactionFee          uint64
+	statusChangeFirstRound  uint64
+	statusChangeValidRounds uint64
+	statusChangeTxFile      string
+	roundFirstValid         uint64
+	roundLastValid          uint64
+	keyDilution             uint64
+	threshold               uint8
+	partKeyOutDir           string
+	partKeyFile             string
+	partKeyDeleteInput      bool
+	importDefault           bool
+	mnemonic                string
 )
 
 func init() {
@@ -77,6 +77,7 @@ func init() {
 	accountCmd.AddCommand(exportCmd)
 	accountCmd.AddCommand(importRootKeysCmd)
 	accountCmd.AddCommand(accountMultisigCmd)
+	accountCmd.AddCommand(markNonparticipatingCmd)
 
 	accountMultisigCmd.AddCommand(newMultisigCmd)
 	accountMultisigCmd.AddCommand(deleteMultisigCmd)
@@ -126,9 +127,9 @@ func init() {
 	changeOnlineCmd.Flags().BoolVarP(&online, "online", "o", true, "Set this account to online or offline")
 	changeOnlineCmd.MarkFlagRequired("online")
 	changeOnlineCmd.Flags().Uint64VarP(&transactionFee, "fee", "f", 0, "The Fee to set on the status change transaction (defaults to suggested fee)")
-	changeOnlineCmd.Flags().Uint64VarP(&onlineFirstRound, "firstRound", "", 0, "FirstValid for the status change transaction (0 for current)")
-	changeOnlineCmd.Flags().Uint64VarP(&onlineValidRounds, "validRounds", "v", 0, "The validity period for the status change transaction")
-	changeOnlineCmd.Flags().StringVarP(&onlineTxFile, "txfile", "t", "", "Write status change transaction to this file")
+	changeOnlineCmd.Flags().Uint64VarP(&statusChangeFirstRound, "firstRound", "", 0, "FirstValid for the status change transaction (0 for current)")
+	changeOnlineCmd.Flags().Uint64VarP(&statusChangeValidRounds, "validRounds", "v", 0, "The validity period for the status change transaction")
+	changeOnlineCmd.Flags().StringVarP(&statusChangeTxFile, "txfile", "t", "", "Write status change transaction to this file")
 	changeOnlineCmd.Flags().BoolVarP(&noWaitAfterSend, "no-wait", "N", false, "Don't wait for transaction to commit")
 
 	// addParticipationKey flags
@@ -170,6 +171,14 @@ func init() {
 	renewAllParticipationKeyCmd.MarkFlagRequired("roundLastValid")
 	renewAllParticipationKeyCmd.Flags().Uint64VarP(&keyDilution, "keyDilution", "", 0, "Key dilution for two-level participation keys")
 	renewAllParticipationKeyCmd.Flags().BoolVarP(&noWaitAfterSend, "no-wait", "N", false, "Don't wait for transaction to commit")
+
+	// markNonparticipatingCmd flags
+	markNonparticipatingCmd.Flags().StringVarP(&accountAddress, "address", "a", "", "Account address to change")
+	markNonparticipatingCmd.Flags().Uint64VarP(&transactionFee, "fee", "f", 0, "The Fee to set on the status change transaction (defaults to suggested fee)")
+	markNonparticipatingCmd.Flags().Uint64VarP(&statusChangeFirstRound, "firstRound", "", 0, "FirstValid for the status change transaction (0 for current)")
+	markNonparticipatingCmd.Flags().Uint64VarP(&statusChangeValidRounds, "validRounds", "v", 0, "The validity period for the status change transaction")
+	markNonparticipatingCmd.Flags().StringVarP(&statusChangeTxFile, "txfile", "t", "", "Write status change transaction to this file, rather than posting to network")
+	markNonparticipatingCmd.Flags().BoolVarP(&noWaitAfterSend, "no-wait", "N", false, "Don't wait for transaction to commit")
 }
 
 var accountCmd = &cobra.Command{
@@ -509,7 +518,6 @@ var changeOnlineCmd = &cobra.Command{
 			os.Exit(1)
 		}
 
-		// Pull the current round for use in our new transactions
 		dataDir := ensureSingleDataDir()
 		client := ensureFullClient(dataDir)
 
@@ -533,7 +541,7 @@ var changeOnlineCmd = &cobra.Command{
 			}
 		}
 
-		err := changeAccountOnlineStatus(accountAddress, part, online, onlineTxFile, walletName, onlineFirstRound, onlineValidRounds, transactionFee, dataDir, client)
+		err := changeAccountOnlineStatus(accountAddress, part, online, statusChangeTxFile, walletName, statusChangeFirstRound, statusChangeValidRounds, transactionFee, dataDir, client)
 		if err != nil {
 			reportErrorf(err.Error())
 		}
@@ -1040,3 +1048,45 @@ var partkeyInfoCmd = &cobra.Command{
 		})
 	},
 }
+
+var markNonparticipatingCmd = &cobra.Command{
+	Use:   "marknonparticipating",
+	Short: "Permanently mark an account as not participating (i.e. offline and earns no rewards)",
+	Long:  "Permanently mark an account as not participating (as opposed to Online or Offline). Once marked, the account can never go online or offline, it is forever nonparticipating, and it will never earn rewards on its balance.",
+	Args:  validateNoPosArgsFn,
+	Run: func(cmd *cobra.Command, args []string) {
+
+		dataDir := ensureSingleDataDir()
+		client := ensureFullClient(dataDir)
+		utx, err := client.MakeUnsignedBecomeNonparticipatingTx(accountAddress, statusChangeFirstRound, statusChangeValidRounds, transactionFee)
+		if err != nil {
+			reportErrorf(errorConstructingTX, err)
+		}
+
+		if statusChangeTxFile != "" {
+			err = writeTxnToFile(client, false, dataDir, walletName, utx, statusChangeTxFile)
+			if err != nil {
+				reportErrorf(fileWriteError, statusChangeTxFile, err)
+			}
+			return
+		}
+
+		// Sign & broadcast the transaction
+		wh, pw := ensureWalletHandleMaybePassword(dataDir, walletName, true)
+		txid, err := client.SignAndBroadcastTransaction(wh, pw, utx)
+		if err != nil {
+			reportErrorf(errorOnlineTX, err)
+		}
+		fmt.Printf("Transaction id for mark-nonparticipating transaction: %s\n", txid)
+
+		if noWaitAfterSend {
+			fmt.Println("Note: status will not change until transaction is finalized")
+			return
+		}
+
+		err = waitForCommit(client, txid)
+		if err != nil {
+			reportErrorf("error waiting for transaction to be committed: %v", err)
+		}
+	},
+}
diff --git a/config/config.go b/config/config.go
@@ -180,6 +180,9 @@ type ConsensusParams struct {
 	// domain-separated credentials
 	CredentialDomainSeparationEnabled bool
 
+	// support for transactions that mark an account non-participating
+	SupportBecomeNonParticipatingTransactions bool
+
 	// fix the rewards calculation by avoiding subtracting too much from the rewards pool
 	PendingResidueRewards bool
 
@@ -409,6 +412,7 @@ func initConsensusProtocols() {
 	vFuture.MaxAssetsPerAccount = 1000
 	vFuture.SupportTxGroups = true
 	vFuture.MaxTxGroupSize = 16
+	vFuture.SupportBecomeNonParticipatingTransactions = true
 	vFuture.ApprovedUpgrades = map[protocol.ConsensusVersion]bool{}
 	Consensus[protocol.ConsensusFuture] = vFuture
 }

diff --git a/data/pools/transactionPool.go b/data/pools/transactionPool.go
@@ -47,17 +47,17 @@ type TransactionPool struct {
 	logStats               bool
 
 	// pendingMu protects pendingTxGroups and pendingTxids
-	pendingMu    deadlock.RWMutex
-	pendingTxGroups  [][]transactions.SignedTxn
-	pendingTxids map[transactions.Txid]transactions.SignedTxn
+	pendingMu       deadlock.RWMutex
+	pendingTxGroups [][]transactions.SignedTxn
+	pendingTxids    map[transactions.Txid]transactions.SignedTxn
 
 	// Calls to remember() add transactions to rememberedTxGroups and
 	// rememberedTxids.  Calling rememberCommit() adds them to the
 	// pendingTxGroups and pendingTxids.  This allows us to batch the
 	// changes in OnNewBlock() without preventing a concurrent call
 	// to Pending() or Verified().
-	rememberedTxGroups  [][]transactions.SignedTxn
-	rememberedTxids map[transactions.Txid]transactions.SignedTxn
+	rememberedTxGroups [][]transactions.SignedTxn
+	rememberedTxids    map[transactions.Txid]transactions.SignedTxn
 }
 
 // MakeTransactionPool is the constructor, it uses Ledger to ensure that no account has pending transactions that together overspend.

diff --git a/data/transactions/keyreg.go b/data/transactions/keyreg.go
@@ -27,11 +27,12 @@ import (
 type KeyregTxnFields struct {
 	_struct struct{} `codec:",omitempty,omitemptyarray"`
 
-	VotePK          crypto.OneTimeSignatureVerifier `codec:"votekey"`
-	SelectionPK     crypto.VRFVerifier              `codec:"selkey"`
-	VoteFirst       basics.Round                    `codec:"votefst"`
-	VoteLast        basics.Round                    `codec:"votelst"`
-	VoteKeyDilution uint64                          `codec:"votekd"`
+	VotePK           crypto.OneTimeSignatureVerifier `codec:"votekey"`
+	SelectionPK      crypto.VRFVerifier              `codec:"selkey"`
+	VoteFirst        basics.Round                    `codec:"votefst"`
+	VoteLast         basics.Round                    `codec:"votelst"`
+	VoteKeyDilution  uint64                          `codec:"votekd"`
+	Nonparticipation bool                            `codec:"nonpart"`
 }
 
 // Apply changes the balances according to this transaction.
@@ -51,11 +52,20 @@ func (keyreg KeyregTxnFields) apply(header Header, balances Balances, spec Speci
 		return fmt.Errorf("cannot change online/offline status of non-participating account %v", header.Sender)
 	}
 
-	// Update the registered keys and mark account as online (or, if the voting or selection keys are zero, offline)
+	// Update the registered keys and mark account as online
+	// (or, if the voting or selection keys are zero, offline/not-participating)
 	record.VoteID = keyreg.VotePK
 	record.SelectionID = keyreg.SelectionPK
 	if (keyreg.VotePK == crypto.OneTimeSignatureVerifier{} || keyreg.SelectionPK == crypto.VRFVerifier{}) {
-		record.Status = basics.Offline
+		if keyreg.Nonparticipation {
+			if balances.ConsensusParams().SupportBecomeNonParticipatingTransactions {
+				record.Status = basics.NotParticipating
+			} else {
+				return fmt.Errorf("transaction tries to mark an account as nonparticipating, but that transaction is not supported")
+			}
+		} else {
+			record.Status = basics.Offline
+		}
 		record.VoteFirstValid = 0
 		record.VoteLastValid = 0
 		record.VoteKeyDilution = 0

diff --git a/data/transactions/keyreg_test.go b/data/transactions/keyreg_test.go
@@ -85,8 +85,16 @@ func TestKeyregApply(t *testing.T) {
 	_, err = tx.Apply(mockBal, SpecialAddresses{FeeSink: feeSink}, 0)
 	require.NoError(t, err)
 
-	// Nonparticipatory accounts should not be able to change status
-	mockBal.addrs[src] = basics.BalanceRecord{Addr: src, AccountData: basics.AccountData{Status: basics.NotParticipating}}
-	_, err = tx.Apply(mockBal, SpecialAddresses{FeeSink: feeSink}, 0)
-	require.Error(t, err)
+	// Going from online to nonparticipatory should be okay, if the protocol supports that
+	if mockBal.ConsensusParams().SupportBecomeNonParticipatingTransactions {
+		tx.KeyregTxnFields = KeyregTxnFields{}
+		tx.KeyregTxnFields.Nonparticipation = true
+		_, err = tx.Apply(mockBal, SpecialAddresses{FeeSink: feeSink}, 0)
+		require.NoError(t, err)
+
+		// Nonparticipatory accounts should not be able to change status
+		mockBal.addrs[src] = basics.BalanceRecord{Addr: src, AccountData: basics.AccountData{Status: basics.NotParticipating}}
+		_, err = tx.Apply(mockBal, SpecialAddresses{FeeSink: feeSink}, 0)
+		require.Error(t, err)
+	}
 }
diff --git a/data/transactions/transaction.go b/data/transactions/transaction.go
@@ -254,7 +254,19 @@ func (tx Transaction) WellFormed(spec SpecialAddresses, proto config.ConsensusPa
 		}
 
 	case protocol.KeyRegistrationTx:
-		// All OK
+		// check that, if this tx is marking an account nonparticipating,
+		// it supplies no key (as though it were trying to go offline)
+		if tx.KeyregTxnFields.Nonparticipation {
+			if !proto.SupportBecomeNonParticipatingTransactions {
+				// if the transaction has the Nonparticipation flag high, but the protocol does not support
+				// that type of transaction, it is invalid.
+				return fmt.Errorf("transaction tries to mark an account as nonparticipating, but that transaction is not supported")
+			}
+			suppliesNullKeys := tx.KeyregTxnFields.VotePK == crypto.OneTimeSignatureVerifier{} || tx.KeyregTxnFields.SelectionPK == crypto.VRFVerifier{}
+			if !suppliesNullKeys {
+				return fmt.Errorf("transaction tries to register keys to go online, but nonparticipatory flag is set")
+			}
+		}
 
 	case protocol.AssetConfigTx:
 		if !proto.Asset {

diff --git a/data/transactions/transaction_test.go b/data/transactions/transaction_test.go
@@ -51,3 +51,70 @@ func TestTransaction_EstimateEncodedSize(t *testing.T) {
 
 	require.Equal(t, 200, tx.EstimateEncodedSize())
 }
+
+func generateDummyGoNonparticpatingTransaction(addr basics.Address) (tx Transaction) {
+	buf := make([]byte, 10)
+	crypto.RandBytes(buf[:])
+
+	proto := config.Consensus[protocol.ConsensusCurrentVersion]
+	tx = Transaction{
+		Type: protocol.KeyRegistrationTx,
+		Header: Header{
+			Sender:     addr,
+			Fee:        basics.MicroAlgos{Raw: proto.MinTxnFee},
+			FirstValid: 1,
+			LastValid:  300,
+		},
+		KeyregTxnFields: KeyregTxnFields{Nonparticipation: true},
+	}
+	tx.KeyregTxnFields.VoteFirst = 1
+	tx.KeyregTxnFields.VoteLast = 300
+	tx.KeyregTxnFields.VoteKeyDilution = 1
+
+	tx.KeyregTxnFields.Nonparticipation = true
+	return tx
+}
+
+func TestGoOnlineGoNonparticipatingContradiction(t *testing.T) {
+	// addr has no significance here other than being a normal valid address
+	addr, err := basics.UnmarshalChecksumAddress("NDQCJNNY5WWWFLP4GFZ7MEF2QJSMZYK6OWIV2AQ7OMAVLEFCGGRHFPKJJA")
+	require.NoError(t, err)
+
+	tx := generateDummyGoNonparticpatingTransaction(addr)
+	// Generate keys, they don't need to be good or secure, just present
+	v := crypto.GenerateOneTimeSignatureSecrets(1, 1)
+	// Also generate a new VRF key
+	vrf := crypto.GenerateVRFSecrets()
+	tx.KeyregTxnFields = KeyregTxnFields{
+		VotePK:           v.OneTimeSignatureVerifier,
+		SelectionPK:      vrf.PK,
+		Nonparticipation: true,
+	}
+	// this tx tries to both register keys to go online, and mark an account as non-participating.
+	// it is not well-formed.
+	feeSink := basics.Address{0x7, 0xda, 0xcb, 0x4b, 0x6d, 0x9e, 0xd1, 0x41, 0xb1, 0x75, 0x76, 0xbd, 0x45, 0x9a, 0xe6, 0x42, 0x1d, 0x48, 0x6d, 0xa3, 0xd4, 0xef, 0x22, 0x47, 0xc4, 0x9, 0xa3, 0x96, 0xb8, 0x2e, 0xa2, 0x21}
+	err = tx.WellFormed(SpecialAddresses{FeeSink: feeSink}, config.Consensus[protocol.ConsensusCurrentVersion])
+	require.Error(t, err)
+}
+
+func TestGoNonparticipatingWellFormed(t *testing.T) {
+	// addr has no significance here other than being a normal valid address
+	addr, err := basics.UnmarshalChecksumAddress("NDQCJNNY5WWWFLP4GFZ7MEF2QJSMZYK6OWIV2AQ7OMAVLEFCGGRHFPKJJA")
+	require.NoError(t, err)
+
+	tx := generateDummyGoNonparticpatingTransaction(addr)
+	curProto := config.Consensus[protocol.ConsensusCurrentVersion]
+
+	if !curProto.SupportBecomeNonParticipatingTransactions {
+		t.Skipf("Skipping rest of test because current protocol version %v does not support become-nonparticipating transactions", protocol.ConsensusCurrentVersion)
+	}
+
+	// this tx is well-formed
+	feeSink := basics.Address{0x7, 0xda, 0xcb, 0x4b, 0x6d, 0x9e, 0xd1, 0x41, 0xb1, 0x75, 0x76, 0xbd, 0x45, 0x9a, 0xe6, 0x42, 0x1d, 0x48, 0x6d, 0xa3, 0xd4, 0xef, 0x22, 0x47, 0xc4, 0x9, 0xa3, 0x96, 0xb8, 0x2e, 0xa2, 0x21}
+	err = tx.WellFormed(SpecialAddresses{FeeSink: feeSink}, curProto)
+	require.NoError(t, err)
+	// but it should stop being well-formed if the protocol does not support it
+	curProto.SupportBecomeNonParticipatingTransactions = false
+	err = tx.WellFormed(SpecialAddresses{FeeSink: feeSink}, curProto)
+	require.Error(t, err)
+}
diff --git a/data/txHandler.go b/data/txHandler.go
@@ -45,7 +45,7 @@ var transactionMessagesDroppedFromPool = metrics.MakeCounter(metrics.Transaction
 // The txBacklogMsg structure used to track a single incoming transaction from the gossip network,
 type txBacklogMsg struct {
 	rawmsg            *network.IncomingMessage      // the raw message from the network
-	unverifiedTxGroup []transactions.SignedTxn       // the unverified ( and signed ) transaction group
+	unverifiedTxGroup []transactions.SignedTxn      // the unverified ( and signed ) transaction group
 	proto             config.ConsensusParams        // the consensus parameters that corresponds to the latest round. Filled in during checkAlreadyCommitted execution.
 	spec              transactions.SpecialAddresses // corresponds to the latest round
 	verificationErr   error                         // The verification error generated by the verification function, if any.
@@ -232,7 +232,7 @@ func (handler *TxHandler) processIncomingTxn(rawmsg network.IncomingMessage) net
 
 	select {
 	case handler.backlogQueue <- &txBacklogMsg{
-		rawmsg:        &rawmsg,
+		rawmsg:            &rawmsg,
 		unverifiedTxGroup: unverifiedTxGroup,
 	}:
 	default: