prioritisetransaction RPC: Restore compatibility with existing implementations by using satoshis for fee offset rather than BTC #5398

1. 
   luke-jr commented at 12:54 PM on December 1, 2014: member

   #4531 was merged changing the unit from satoshis to RPC on the premise of prioritisetransaction being a newly introduced RPC, but in fact it has been around and in use out-of-tree since v0.6.1. I don't think we should bring it into 0.10 incompatibly.

2. 
   sipa commented at 1:43 PM on December 1, 2014: member

   Don't care strongly, but untested ACK. It makes more sense to mimick GBT here than wallet RPCs.

3. 
   laanwj commented at 1:51 PM on December 1, 2014: member

   utACK, I hate flip-flopping code and this makes the interface seeming less consistent, but currently the RPC call doesn't even work (as AmountFromValue doesn't accept negative values). Can you add a comment that satoshis are to be used in mining-related RPCs to avoid that someone changes this back yet again?

4. prioritisetransaction RPC: Restore compatibility with existing implementations by using satoshis for fee offset rather than BTC 8a20cd3c51
5. luke-jr force-pushed on Dec 1, 2014
6. laanwj added this to the milestone 0.10.0 on Dec 2, 2014
7. laanwj added the label Block Generation on Dec 2, 2014
8. luke-jr cross-referenced this on Dec 3, 2014 from issue qa/rpc-tests: Test prioritisetransaction by luke-jr
9. laanwj merged this on Dec 5, 2014
10. laanwj closed this on Dec 5, 2014

    ---

11. laanwj referenced this in commit dbf1dc239e on Dec 5, 2014
12. in src/rpcmining.cpp:None in 8a20cd3c51

     291 | +            + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000")
     292 |          );
     293 |  
     294 |      uint256 hash;
     295 |      hash.SetHex(params[0].get_str());
     296 |  
    

    ---

    ---

    

    SergioDemianLerner commented at 9:21 PM on December 16, 2014:

    Instead of SetHex, a function similar to ParseHashStr() should be used to report invalid values.

13. in src/rpcmining.cpp:None in 8a20cd3c51

     297 | -    CAmount nAmount = 0;
     298 | -    if (params[2].get_real() != 0.0)
     299 | -        nAmount = AmountFromValue(params[2]);
     300 | +    CAmount nAmount = params[2].get_int64();
     301 |  
     302 |      mempool.PrioritiseTransaction(hash, params[0].get_str(), params[1].get_real(), nAmount);
    

    ---

    ---

    

    SergioDemianLerner commented at 9:28 PM on December 16, 2014:

    params[0].get_str() is passed to PrioritiseTransaction() without filter. Then this is logged into the logfile without filter. This can be used to crash a logfile parser application, to embed malicius javascript, to add confusing log messages, etc. params[0] should not be passed, since hash already can be converted into an hex string for logging.

14. luke-jr cross-referenced this on Dec 17, 2014 from issue Bugfix: prioritisetransaction: Do some basic sanity checking on txid by luke-jr
15. luke-jr deleted the branch on Jan 1, 2015
16. langerhans cross-referenced this on Mar 16, 2015 from issue Add helptexts for -whitelist and rpc prioritisetransaction and switch - by Sporklin
17. bitcoin locked this on Sep 8, 2021

Contributors

luke-jrsipalaanwjSergioDemianLerner

Milestone
0.10.0