Bitcoin Forum
September 28, 2016, 10:10:22 PM *
News: Due to DDoS attacks, there may be periodic downtime.
 
   Home   Help Search Donate Login Register  
Pages: [1]
  Print  
Author Topic: Proposed change to sendtoaddress API call  (Read 4442 times)
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
August 13, 2010, 07:28:23 PM
 #1

I'm proposing one small change to Bitcoin's JSON-RPC api:  return a transaction ID when Bitcoins are successfully sent.

Why?  Because I want to keep a complete audit trail for any coins going into or coming out of my application's wallet; I want to keep track of the particular transactions in the bitcoin network that correspond to actions my application takes.  The alternative is to call sendtoaddress and then call listtransactions, but that won't work properly if two similar transactions (same amount to same address) occur at about the same time.

So I propose doing the simplest thing possible: modify the JSON-RPC sendtoaddress call so it returns the string 'sent:' followed by the 256-bit-hexadecimal transactions id.

This could break any applications that look for exactly the string 'sent' (which is what sendtoaddress does now).  The fix would be to modify the apps to see if the string began with 'sent'.

Alternatives I thought about but think I don't like:
 + make it a new api call so old apps do not break (sendtoaddress2 ? yuck)
 + return just the transaction id on successful send instead of 'sent:...'
 + return an array with more information (maybe [ "tx_id": "...", "fee" : 0.0 ] )

Comments/criticisms?

How often do you get the chance to work on a potentially world-changing project?
1475100622
Hero Member
*
Offline Offline

Posts: 1475100622

View Profile Personal Message (Offline)

Ignore
1475100622
Reply with quote  #2

1475100622
Report to moderator
1475100622
Hero Member
*
Offline Offline

Posts: 1475100622

View Profile Personal Message (Offline)

Ignore
1475100622
Reply with quote  #2

1475100622
Report to moderator
1475100622
Hero Member
*
Offline Offline

Posts: 1475100622

View Profile Personal Message (Offline)

Ignore
1475100622
Reply with quote  #2

1475100622
Report to moderator
Advertised sites are not endorsed by the Bitcoin Forum. They may be unsafe, untrustworthy, or illegal in your jurisdiction. Advertise here.
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
August 13, 2010, 07:40:20 PM
 #2


100% agreed with your suggestion, about the need for txn_id association.

However, it seems better to avoid API breakage, because this API is so heavily used.  It is almost guaranteed that anyone automating bitcoins is using sendtoaddress.

Either create a "send2" RPC call, or add an "[extended-JSON=0]" parameter to the existing sendtoaddress RPC call, or something similar that preserves existing application data flows.



Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
knightmb
Sr. Member
****
Offline Offline

Activity: 308


Timekoin - Save Electricity, Don't Waste It!


View Profile WWW
August 13, 2010, 07:41:14 PM
 #3

Just add a flag (like true/false) to the function with the default "false" so it works with older uses of the function call.

Like

sendtoaddress <bitcoinaddress> <amount> [comment] [comment-to] [return transaction ID, default false]

So
sendtoaddress 1FtDzyajiHKa9QbXiNxqztB 1.00 "Blah Comment" "Blah From" true
returns
sent XYZ....

But
sendtoaddress 1FtDzyajiHKa9QbXiNxqztB 1.00 "Blah Comment" "Blah From"
returns
sent
Also without having to break compatibility.

I think it would be a handy addition.

Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
August 13, 2010, 08:01:17 PM
 #4

RE: adding a flag:  great idea!

If you set the flag, I don't see any reason to prepend 'sent' to the transaction ID; better to just return the transaction ID on successful send.

Patches:
Code:
diff --git a/rpc.cpp b/rpc.cpp
index 920fe90..8714b7e 100644
--- a/rpc.cpp
+++ b/rpc.cpp
@@ -342,10 +342,11 @@ Value getaddressesbylabel(const Array& params, bool fHelp)
 
 Value sendtoaddress(const Array& params, bool fHelp)
 {
-    if (fHelp || params.size() < 2 || params.size() > 4)
+    if (fHelp || params.size() < 2 || params.size() > 5)
         throw runtime_error(
-            "sendtoaddress <bitcoinaddress> <amount> [comment] [comment-to]\n"
-            "<amount> is a real and is rounded to the nearest 0.01");
+            "sendtoaddress <bitcoinaddress> <amount> [comment] [comment-to] [return-tx-id-flag]\n"
+            "<amount> is a real and is rounded to the nearest 0.01\n"
+            "returns string 'sent' if return-tx-id-flag is false (default), otherwise returns transaction id.");
 
     string strAddress = params[0].get_str();
 
@@ -361,9 +362,15 @@ Value sendtoaddress(const Array& params, bool fHelp)
     if (params.size() > 3 && params[3].type() != null_type && !params[3].get_str().empty())
         wtx.mapValue["to"]      = params[3].get_str();
 
+    bool fReturnTxID = false;
+    if (params.size() > 4)
+        fReturnTxID = params[4].get_bool();
+
     string strError = SendMoneyToBitcoinAddress(strAddress, nAmount, wtx);
     if (strError != "")
         throw runtime_error(strError);
+    if (fReturnTxID)
+        return wtx.GetHash().ToString();
     return "sent";
 }
 
@@ -1103,6 +1110,7 @@ int CommandLineRPC(int argc, char *argv[])
         if (strMethod == "setgenerate"            && n > 0) ConvertTo<bool>(params[0]);
         if (strMethod == "setgenerate"            && n > 1) ConvertTo<boost::int64_t>(params[1]);
         if (strMethod == "sendtoaddress"          && n > 1) ConvertTo<double>(params[1]);
+        if (strMethod == "sendtoaddress"          && n > 4) ConvertTo<bool>(params[4]);
         if (strMethod == "listtransactions"       && n > 0) ConvertTo<boost::int64_t>(params[0]);
         if (strMethod == "listtransactions"       && n > 1) ConvertTo<bool>(params[1]);
         if (strMethod == "getamountreceived"      && n > 1) ConvertTo<boost::int64_t>(params[1]); // deprecated

How often do you get the chance to work on a potentially world-changing project?
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
August 13, 2010, 08:13:30 PM
 #5


What happens when we desire to return additional information, beyond tx-id?

For the sake of future compatibility, it seems like the flag should present a choice between returning (a) just the current 'sent', or (b) a JSON map containing tx-id, and perhaps other things.


Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
August 13, 2010, 08:26:06 PM
 #6

What happens when we desire to return additional information, beyond tx-id?

For the sake of future compatibility, it seems like the flag should present a choice between returning (a) just the current 'sent', or (b) a JSON map containing tx-id, and perhaps other things.
A 'gettransaction tx_id' API call is on my short list.

What do other folks think; should sendtoaddress .... true   return just the tx_id, and you have to make another API call to get details if you need them?
Or should it return an Array?

How often do you get the chance to work on a potentially world-changing project?
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
August 13, 2010, 11:01:14 PM
 #7


Do you need 'gettransaction', given the existence of 'getblock'?


Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
satoshi
Founder
Sr. Member
*
qt
Offline Offline

Activity: 364


View Profile
August 13, 2010, 11:39:14 PM
 #8

It's too soon to start junking up the API for backward compatibility at all costs.

Just return "<txid>".
knightmb
Sr. Member
****
Offline Offline

Activity: 308


Timekoin - Save Electricity, Don't Waste It!


View Profile WWW
August 13, 2010, 11:40:37 PM
 #9

What happens when we desire to return additional information, beyond tx-id?

For the sake of future compatibility, it seems like the flag should present a choice between returning (a) just the current 'sent', or (b) a JSON map containing tx-id, and perhaps other things.
A 'gettransaction tx_id' API call is on my short list.

What do other folks think; should sendtoaddress .... true   return just the tx_id, and you have to make another API call to get details if you need them?
Or should it return an Array?

Depends on how fancy you want to be.

If you go beyond a boolean, you could just append flags for any reason or function needed. This would leave it open to more functions rather than stacking a ton of booleans on the function as it's usefulness grows.

sendtoaddress <bitcoinaddress> <amount> [comment] [comment-to]
-tx_id : returns the tx ID of the receiving bitcoin address
-dance : returns dance high scores
-moon : return current moon phase

So one would simply put in
sendtoaddress -tx-_id <bitcoinaddress> <amount> [comment] [comment-to]
instead of
sendtoaddress <bitcoinaddress> <amount> [comment] [comment-to] true false true true false
if a lot of functions were added to it later.

Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
August 14, 2010, 01:46:17 PM
 #10

It's too soon to start junking up the API for backward compatibility at all costs.
Just return "<txid>".
You'll never hear me argue with "keep it simple." 

Patch for that is trivial:
Code:
diff --git a/rpc.cpp b/rpc.cpp
index 920fe90..35a336f 100644
--- a/rpc.cpp
+++ b/rpc.cpp
@@ -364,7 +364,7 @@ Value sendtoaddress(const Array& params, bool fHelp)
     string strError = SendMoneyToBitcoinAddress(strAddress, nAmount, wtx);
     if (strError != "")
         throw runtime_error(strError);
-    return "sent";
+    return wtx.GetHash().ToString();
 }

How often do you get the chance to work on a potentially world-changing project?
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
August 14, 2010, 06:11:20 PM
 #11


I think it was implied to be

     return "<" + wtx.GetHash().ToString() + ">";

?


Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
August 14, 2010, 11:57:01 PM
 #12

Nah, satoshi uses <foo> to mean "replace this placeholder with the actual thingy."

It'd be annoying to constantly strip the < and > from the txid.

How often do you get the chance to work on a potentially world-changing project?
xane
Jr. Member
*
Offline Offline

Activity: 45



View Profile WWW
July 15, 2011, 04:55:42 PM
 #13

Was this ever done? Would be extremely useful to my application as well...

https://cardsforbitcoins.com - Instantly exchange Bitcoins for Amazon gift cards
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
July 15, 2011, 11:51:15 PM
 #14

Yes, all of the send* methods return a transaction id on successful send.

How often do you get the chance to work on a potentially world-changing project?
Pages: [1]
  Print  
 
Jump to:  

Sponsored by , a Bitcoin-accepting VPN.
Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!