Skip to content

Conversation

@furszy
Copy link

@furszy furszy commented Oct 14, 2021

Straightforward, implements #2599 functionality.

@furszy furszy self-assigned this Oct 14, 2021
@Fuzzbawls
Copy link
Collaborator

Index: src/wallet/rpcwallet.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
--- a/src/wallet/rpcwallet.cpp	(revision c1056fd213d7162404c008b240a193d7d9b781f2)
+++ b/src/wallet/rpcwallet.cpp	(date 1634195580801)
@@ -555,7 +555,7 @@
     if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
         return NullUniValue;
 
-    if (request.fHelp || !request.params.empty())
+    if (request.fHelp || request.params.size() > 1)
         throw std::runtime_error(
                 "getnewshieldaddress ( \"label\" )\n"
                 "\nReturns a new shield address for receiving payments.\n"
@@ -575,8 +575,8 @@
         );
 
     std::string label;
-    if (!params.isNull() && params.size() > 0) {
-        label = LabelFromValue(params[0]);
+    if (!request.params.empty()) {
+        label = LabelFromValue(request.params[0]);
     }
 
     LOCK2(cs_main, pwallet->cs_wallet);

@Fuzzbawls Fuzzbawls added Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes RPC labels Oct 14, 2021
@random-zebra
Copy link

Also, we need to support named args for the RPC now

Index: src/wallet/rpcwallet.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
--- a/src/wallet/rpcwallet.cpp	(revision c1056fd213d7162404c008b240a193d7d9b781f2)
+++ b/src/wallet/rpcwallet.cpp	(date 1634214619918)
@@ -4787,7 +4787,7 @@
     { "wallet",             "bip38decrypt",             &bip38decrypt,             true,  {"encrypted_key","passphrase"} },
 
     /** Sapling functions */
-    { "wallet",             "getnewshieldaddress",           &getnewshieldaddress,            true,  {} },
+    { "wallet",             "getnewshieldaddress",           &getnewshieldaddress,            true,  {"label"} },
     { "wallet",             "listshieldaddresses",           &listshieldaddresses,            false, {"include_watchonly"} },
     { "wallet",             "exportsaplingkey",              &exportsaplingkey,               true,  {"shield_addr"} },
     { "wallet",             "importsaplingkey",              &importsaplingkey,               true,  {"key","rescan","height"} },

@furszy furszy force-pushed the 2021_getnewshieldaddr_add_label branch from c1056fd to 5718ad8 Compare October 14, 2021 23:01
@furszy furszy added this to the 5.4.0 milestone Oct 14, 2021
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 5718ad8

@furszy furszy modified the milestones: 5.4.0, 6.0.0 Oct 16, 2021
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 5718ad8

@furszy furszy merged commit c1193cc into PIVX-Project:master Oct 19, 2021
furszy added a commit that referenced this pull request Nov 19, 2021
db68b90 [RPC] Shield address setlabel/getaddressesbylabel (Fuzzbawls)

Pull request description:

  This adds shield address support to the `setlabel` and
  `getaddressesbylabel` RPC commands. `setlabel` now accepts and validates
   shield address input argument, where `getaddressesbylabel` now returns
   shield addresses with a label matching the input argument.

  Companion to #2600 that allows CLI users to set a label for shield addresses
  after-the-fact, as well as see shield addresses with an assigned label in the
  response of `getaddressesbylabel`

  Also added relevant release notes for the changes in #2600.

  Regression tests added as well to cover the `setlabel` command.

ACKs for top commit:
  random-zebra:
    ACK db68b90
  furszy:
    code ACK db68b90, merging..

Tree-SHA512: 3344849a612c36eddd00ff30110b7cf48b044976dc1312c8b2769c8df33d8ffd971e5a4142424cbaabd90ea8632b4c9da83a001f90cce044b487a477fe8870c2
@random-zebra random-zebra removed the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Dec 17, 2021
@Fuzzbawls Fuzzbawls modified the milestones: 6.0.0, 5.5.0 Sep 11, 2022
@furszy furszy deleted the 2021_getnewshieldaddr_add_label branch November 29, 2022 14:43
@Fuzzbawls Fuzzbawls added Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes and removed Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes labels Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants