Skip to content

Commit

Permalink
Modify default account naming policy.
Browse files Browse the repository at this point in the history 
Rather than disallowing the default account to be renamed as was
proposed in #245 (and implemented in #246), the default account name
is no longer considered a reserved name by the address manager.
Instead, it is simply the initial name used for the first initial
account.

A database upgrade removes any additional aliases for the default
account in the database.  This prevents a lookup for some name which
is not an account name from mapping to the default account
unexpectedly (potentially preventing incorrect account usage from the
RPC server due to bad iteraction with default parameters).

All unset account names in a JSON-RPC request are expected to be set
nil by btcjson.  This behavior depends on btcsuite/btcd#399.

Additionally, the manager no longer considers the wildcard * to be a
reserved account name.  Due to poor API decisions, the RPC server
overloads the meaning of account fields to optionally allow referring
to all accounts at a time, or a single account.  This is not a address
manager responsibility, though, as a future cleaner API should not use
multiple differet meanings for the same field across multiple
requests.  Therefore, don't burden down future APIs with this quirk
and prevent incorrect wildcard usage from the RPC server.
  • Loading branch information
@jrick
jrick committed Apr 25, 2015
1 parent 67b0597 commit 946ae44
Show file tree
Hide file tree
Showing 8 changed files with 239 additions and 68 deletions.
26 changes: 13 additions & 13 deletions internal/rpchelp/helpdescs_en_US.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ var helpDescsEnUS = map[string]string{
// GetBalanceCmd help.
"getbalance--synopsis": "Calculates and returns the balance of one or all accounts.",
"getbalance-minconf": "Minimum number of block confirmations required before an unspent output's value is included in the balance",
"getbalance-account": "The account name to query the balance for, or '*' to consider all accounts",
"getbalance--condition0": "account!=*",
"getbalance--condition1": "account=*",
"getbalance-account": "The account name to query the balance for, or \"*\" to consider all accounts (default=\"*\")",
"getbalance--condition0": "account != \"*\"",
"getbalance--condition1": "account = \"*\"",
"getbalance--result0": "The balance of 'account' valued in bitcoin",
"getbalance--result1": "The balance of all accounts valued in bitcoin",

Expand Down Expand Up @@ -94,12 +94,12 @@ var helpDescsEnUS = map[string]string{

// GetNewAddressCmd help.
"getnewaddress--synopsis": "Generates and returns a new payment address.",
"getnewaddress-account": "Account name the new address will belong to",
"getnewaddress-account": "Account name the new address will belong to (default=\"default\")",
"getnewaddress--result0": "The payment address",

// GetRawChangeAddressCmd help.
"getrawchangeaddress--synopsis": "Generates and returns a new internal payment address for use as a change address in raw transactions.",
"getrawchangeaddress-account": "Account name the new internal address will belong to",
"getrawchangeaddress-account": "Account name the new internal address will belong to (default=\"default\")",
"getrawchangeaddress--result0": "The internal payment address",

// GetReceivedByAccountCmd help.
Expand All @@ -122,8 +122,8 @@ var helpDescsEnUS = map[string]string{
// HelpCmd help.
"help--synopsis": "Returns one line summaries for every server method or detailed usage of a single request.",
"help-command": "Method to query detailed usage for",
"help--condition0": "command=''",
"help--condition1": "command!=''",
"help--condition0": "command = \"\"",
"help--condition1": "command != \"\"",
"help--result0": "One line usage for every server request",
"help--result1": "Detailed usage of a single request",

Expand Down Expand Up @@ -151,7 +151,7 @@ var helpDescsEnUS = map[string]string{
// ImportPrivKeyCmd help.
"importprivkey--synopsis": "Imports a WIF-encoded private key to the 'imported' account.",
"importprivkey-privkey": "The WIF-encoded private key",
"importprivkey-label": "Unused (all imported addresses belong to the imported account)",
"importprivkey-label": "Unused (must be unset or 'imported')",
"importprivkey-rescan": "Rescan the blockchain (since the genesis block) for outputs controlled by the imported key",

// KeypoolRefillCmd help.
Expand Down Expand Up @@ -227,7 +227,7 @@ var helpDescsEnUS = map[string]string{

// ListTransactionsCmd help.
"listtransactions--synopsis": "Returns a JSON array of objects containing verbose details for wallet transactions.",
"listtransactions-account": "Unused",
"listtransactions-account": "Unused (must be unset or \"*\")",
"listtransactions-count": "Maximum number of transactions to create results from",
"listtransactions-from": "Number of transactions to skip before results are created",
"listtransactions-includewatchonly": "Unused",
Expand Down Expand Up @@ -363,7 +363,7 @@ var helpDescsEnUS = map[string]string{

// ExportWatchingWalletCmd help.
"exportwatchingwallet--synopsis": "Creates and returns a duplicate of the wallet database without any private keys to be used as a watching-only wallet.",
"exportwatchingwallet-account": "Unused",
"exportwatchingwallet-account": "Unused (must be unset or \"*\")",
"exportwatchingwallet-download": "Unused",
"exportwatchingwallet--result0": "The watching-only database encoded as a base64 string",

Expand All @@ -376,17 +376,17 @@ var helpDescsEnUS = map[string]string{

// GetUnconfirmedBalanceCmd help.
"getunconfirmedbalance--synopsis": "Calculates the unspent output value of all unmined transaction outputs for an account.",
"getunconfirmedbalance-account": "The account to query the unconfirmed balance for",
"getunconfirmedbalance-account": "The account to query the unconfirmed balance for (default=\"default\")",
"getunconfirmedbalance--result0": "Total amount of all unmined unspent outputs of the account valued in bitcoin.",

// ListAddressTransactionsCmd help.
"listaddresstransactions--synopsis": "Returns a JSON array of objects containing verbose details for wallet transactions pertaining some addresses.",
"listaddresstransactions-addresses": "Addresses to filter transaction results by",
"listaddresstransactions-account": "Unused",
"listaddresstransactions-account": "Unused (must be unset or \"*\")",

// ListAllTransactionsCmd help.
"listalltransactions--synopsis": "Returns a JSON array of objects in the same format as 'listtransactions' without limiting the number of returned objects.",
"listalltransactions-account": "Unused",
"listalltransactions-account": "Unused (must be unset or \"*\")",

// RenameAccountCmd help.
"renameaccount--synopsis": "Renames an account.",
Expand Down
73 changes: 65 additions & 8 deletions rpcserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ var (
Code: btcjson.ErrRPCNoTxInfo,
Message: "No information for transaction",
}

ErrReservedAccountName = btcjson.RPCError{
Code: btcjson.ErrRPCInvalidParameter,
Message: "Account name is reserved by RPC server",
}
)

// TODO(jrick): There are several error paths which 'replace' various errors
Expand Down Expand Up @@ -1621,6 +1626,15 @@ func DumpWallet(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (int
// current wallet as a watching wallet (with no private keys), and returning
// base64-encoding of serialized account files.
func ExportWatchingWallet(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.ExportWatchingWalletCmd)

if cmd.Account != nil && *cmd.Account != "*" {
return nil, btcjson.RPCError{
Code: btcjson.ErrRPCWallet,
Message: "Individual accounts can not be exported as watching-only",
}
}

return w.ExportWatchingWallet(cfg.WalletPass)
}

Expand Down Expand Up @@ -1656,7 +1670,10 @@ func GetBalance(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (int

var balance btcutil.Amount
var err error
accountName := *cmd.Account
accountName := "*"
if cmd.Account != nil {
accountName = *cmd.Account
}
if accountName == "*" {
balance, err = w.CalculateBalance(int32(*cmd.MinConf))
} else {
Expand Down Expand Up @@ -1795,11 +1812,14 @@ func GetAccountAddress(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{
func GetUnconfirmedBalance(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.GetUnconfirmedBalanceCmd)

account, err := w.Manager.LookupAccount(*cmd.Account)
acctName := "default"
if cmd.Account != nil {
acctName = *cmd.Account
}
account, err := w.Manager.LookupAccount(acctName)
if err != nil {
return nil, err
}

unconfirmed, err := w.CalculateAccountBalance(account, 0)
if err != nil {
return nil, err
Expand All @@ -1820,7 +1840,7 @@ func ImportPrivKey(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (
// Ensure that private keys are only imported to the correct account.
//
// Yes, Label is the account name.
if *cmd.Label != waddrmgr.ImportedAddrAccountName {
if cmd.Label != nil && *cmd.Label != waddrmgr.ImportedAddrAccountName {
return nil, ErrNotImportedAccount
}

Expand Down Expand Up @@ -1863,6 +1883,12 @@ func KeypoolRefill(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (
func CreateNewAccount(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.CreateNewAccountCmd)

// The wildcard * is reserved by the rpc server with the special meaning
// of "all accounts", so disallow naming accounts to this string.
if cmd.Account == "*" {
return nil, ErrReservedAccountName
}

// Check that we are within the maximum allowed non-empty accounts limit.
account, err := w.Manager.LastAccount()
if err != nil {
Expand Down Expand Up @@ -1894,6 +1920,13 @@ func CreateNewAccount(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}
// If the account does not exist an appropiate error will be returned.
func RenameAccount(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.RenameAccountCmd)

// The wildcard * is reserved by the rpc server with the special meaning
// of "all accounts", so disallow naming accounts to this string.
if cmd.NewAccount == "*" {
return nil, ErrReservedAccountName
}

// Check that given account exists
account, err := w.Manager.LookupAccount(cmd.OldAccount)
if err != nil {
Expand All @@ -1910,11 +1943,14 @@ func RenameAccount(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (
func GetNewAddress(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.GetNewAddressCmd)

account, err := w.Manager.LookupAccount(*cmd.Account)
acctName := "default"
if cmd.Account != nil {
acctName = *cmd.Account
}
account, err := w.Manager.LookupAccount(acctName)
if err != nil {
return nil, err
}

addr, err := w.NewAddress(account)
if err != nil {
return nil, err
Expand All @@ -1931,7 +1967,12 @@ func GetNewAddress(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (
// but ignores the parameter.
func GetRawChangeAddress(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.GetRawChangeAddressCmd)
account, err := w.Manager.LookupAccount(*cmd.Account)

acctName := "default"
if cmd.Account != nil {
acctName = *cmd.Account
}
account, err := w.Manager.LookupAccount(acctName)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2436,7 +2477,7 @@ func ListTransactions(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}
// between transactions pertaining to one account from another. This
// will be resolved when wtxmgr is combined with the waddrmgr namespace.

if cmd.Account != nil {
if cmd.Account != nil && *cmd.Account != "*" {
// For now, don't bother trying to continue if the user
// specified an account, since this can't be (easily or
// efficiently) calculated.
Expand All @@ -2457,6 +2498,13 @@ func ListTransactions(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}
func ListAddressTransactions(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.ListAddressTransactionsCmd)

if cmd.Account != nil && *cmd.Account != "*" {
return nil, btcjson.RPCError{
Code: btcjson.ErrRPCInvalidParameter,
Message: "Listing transactions for addresses may only be done for all accounts",
}
}

// Decode addresses.
hash160Map := make(map[string]struct{})
for _, addrStr := range cmd.Addresses {
Expand All @@ -2475,6 +2523,15 @@ func ListAddressTransactions(w *wallet.Wallet, chainSvr *chain.Client, icmd inte
// similar to ListTransactions, except it takes only a single optional
// argument for the account name and replies with all transactions.
func ListAllTransactions(w *wallet.Wallet, chainSvr *chain.Client, icmd interface{}) (interface{}, error) {
cmd := icmd.(*btcjson.ListAllTransactionsCmd)

if cmd.Account != nil && *cmd.Account != "*" {
return nil, btcjson.RPCError{
Code: btcjson.ErrRPCInvalidParameter,
Message: "Listing all transactions may only be done for all accounts",
}
}

return w.ListAllTransactions()
}

Expand Down
Loading

0 comments on commit 946ae44

Please sign in to comment.