[Wallet Feature Request]: Multiwallet-compatible walletnotify

Kexkey commented at 2:31 pm on May 15, 2018: none

Different wallets may be used for different purposes. When a notification is triggered, we get the TXID but don’t know which wallet is involved. It would be useful to have different walletnotify actions depending on which wallet is involved by the transaction.

It could be used like (bitcoin.conf):

walletnotify.walletname=action %s

Or maybe a more simple and flexible (dynamic-wallets compatible) way:

walletnotify=action %s %w

where %w is the name of the wallet. This way, notify action (and not Bitcoin Core) is responsible for choosing the right action depending on the wallet and it is easier to RPC query the good wallet (ie gettransaction).

promag commented at 2:52 pm on May 15, 2018: member

Sounds sensible to add support to replace the 2nd %s with the wallet name.

Concept ACK.

promag commented at 3:13 pm on May 15, 2018: member

Actually it’s preferable to use a different placeholder for the wallet name, like @Kexkey suggests, because currently

0boost::replace_all(strCmd, "%s", wtxIn.GetHash().GetHex());

so this should work:

0boost::replace_all(strCmd, "%w", GetName());

knoxcard commented at 3:33 pm on May 15, 2018: contributor

Just throwing my two cents in, this is how I currently deal with incoming transactions. Others might not be aware of this or just getting started. It could simply adjust someone’s perspective and allow them to create something entirely better.

Incoming Transaction Pseudocode

 0Instant Payment URL Notification
 1-------------------------------------------
 2walletnotify=someurl/btc/:txid
 3[url.get receiver]
 4addtransaction(get txid parameter in url)
 5
 6Background Cron Job
 7--------------------------------------------
 8cron listtransactions every minute
 9iterate through all the txids
10addtransaction(txid)
11
12function addtransaction(txid)
13txdata = gettransaction txid
14if txdata.account = "users_wallet"  
15   add txdata as new record in table user_transactions
16  confirmations = txdata.confirmations
17  trans_amount = txdata.amount
18  ...
19end if
20end function

if txdata.account = "users_wallet" might be better to do if txdata.label = "users_wallet". Noticed those variables always match up, is the account name being deprecated?

Sample JSON RPC Output - gettransaction

 0[
 1  {
 2    "account": "indospace",
 3    "address": "bitcoincash:qq730w3dys53j3msnldex84shw695atvuvh7hj3u3q",
 4    "category": "receive",
 5    "amount": 0.00009754,
 6    "label": "indospace",
 7    "vout": 0,
 8    "confirmations": 234,
 9    "blockhash": "000000000000000000939cd1b4771d1762b86fbac7f89a4cd9618103f7d992d1",
10    "blockindex": 106,
11    "blocktime": 1526261896,
12    "txid": "339c1604a03ca14d956b76037fa71e453e7c1edf1029f7fc94a8bbe3bee38c00",
13    "walletconflicts": [
14    ],
15    "time": 1526261567,
16    "timereceived": 1526261567
17  }
18]

knoxcard commented at 3:49 pm on May 15, 2018: contributor

After taking a good look at this thread, I am leaning towards @Kexkey suggestion. It is could be worth adding primarily due to programming structure, secondarily to ridding an unnecessary iterative process. If you are a busy platform receiving thousands of payments, you still have to iterate through every incoming transaction to determine if it belongs to a particular wallet. Now, what does this cost CPU and memory wise? Maybe not much at all, but I find it kind of ugly in terms of programmatical structure.

fanquake added the label Wallet on May 15, 2018

promag commented at 9:05 am on May 30, 2018: member

Should the wallet name be in ZMQ notifications too?

Kexkey commented at 3:38 pm on June 7, 2018: none

@promag I’d say ZMQ is mostly used for network events (tx and blocks) while walletnotify is tightly related to wallet(s). So I don’t think so.

But I was thinking about the credentials… should there be different authentications for the different wallets? Is that a use case to have one node shared by multiple users/wallets? In that case, the different usernames/passwords would make sense.

I personally would share my nodes with my family members or friends, making sure each one has its auth creds and the wallets are independently encrypted.

isghe commented at 12:14 pm on December 12, 2018: contributor

Instead of using the wallet name, could not we introduce a walletid/walletnamehash?

This to avoid escape/unescape (prone to errors) problems, shown in #13339

EDIT: Of course after that, it will be necessary to introduce new RPC API, and to adapt the existing ones, to use wallethash; e.g:

bitcoin-cli wallethash "walletname" Returns its wallethash “upserting” the association in an internal DB (it could be non persistent DB);
bitcoin-cli walletname wallethash Returns its walletname (previously stored with RPC wallethash);

etc…

promag commented at 12:22 pm on December 12, 2018: member

@isghe I think an alternative is to use environment variables, however the implementation is not that simple.

laanwj closed this on Feb 17, 2020

sidhujag referenced this in commit 0f86b36c7e on Feb 18, 2020

sidhujag referenced this in commit d43bf96cab on Nov 10, 2020

DrahtBot locked this on Feb 15, 2022

[Wallet Feature Request]: Multiwallet-compatible walletnotify #13237