Pages:
Author

Topic: [PULL] monitortx monitorblocks listmonitored getblock (Read 4284 times)

full member
Activity: 175
Merit: 102
Gavin,

I have been running with this patch for a few weeks now (decided to go ahead and see how big the problem might be).

So, orphaned blocks received from the network definitely do get posted via the monitorBlock workflow.  However, when bitcoin eventually re-orgs the block chain to replace the orphaned blocks with the real block from the longest chain, those blocks are not being posted.  For now I have a process that runs once in awhile to detect these cases and updates the values stored in the db, but I'm wondering if something can be modified in the patch to address it.

Interestingly, if the orphaned block is one of my own, I actually want to retain the old value for tracking purposes.

With regard to dependent orphaned transactions, I haven't run into those yet so can't comment.
newbie
Activity: 31
Merit: 0
My C++ is really rusty, and I'm reading the diff out of context, but I believe that this patch just lets you add URLs to two lists, one that gets POSTs for every new block that comes in, and one that gets POSTs for every transaction that comes in related to the running wallet.

I hope this is not the case, and it doesn't make sense.  If you're receiving blocks complete with all transactions and you have addresses in your POST queue for monitortx why would you go out of your way to filter results based on the wallet?  It's an extra step to reduce functionality.

legendary
Activity: 2128
Merit: 1073
... but I'm not 100% happy with it. I'm not sure it properly handles block chain re-orgs and dependent orphan transactions. Would be nice to write some tests to exercise those edge cases, and figure out what it SHOULD do in those cases.
In addition to the above this patch doesn't seem to have any mechanism for garbage collecting stale notification callbacks. From the cursory look I would guess that it doesn't even try to be idempotent, i.e. one could add multiple identical callback URLs with no limit that I could see.
kjj
legendary
Activity: 1302
Merit: 1026
My C++ is really rusty, and I'm reading the diff out of context, but I believe that this patch just lets you add URLs to two lists, one that gets POSTs for every new block that comes in, and one that gets POSTs for every transaction that comes in related to the running wallet.
member
Activity: 76
Merit: 12
From reading your other posts, I get the impression that you want to know about addresses that are not in the wallet.  I don't think this patch gives you that.

Oh doesn't it?  That was my understanding, as posted above, and no one has corrected me on that yet.

j
kjj
legendary
Activity: 1302
Merit: 1026
From reading your other posts, I get the impression that you want to know about addresses that are not in the wallet.  I don't think this patch gives you that.
newbie
Activity: 31
Merit: 0
This thread covers several different things.  Exactly which features are you offering the bounty for?

Well it's the entire pull request we where thinking of (i.e. push based notification for transactions and blocks, plus generic getblock).  So anyone who contributes to the finalizing of this pull request, once it's actually integrated with the client.

After some additional consideration we are not offering this bounty for a solution that is not integrated with bitcoind.  The goal is to get the feature we want and promote it for inclusion in the client so the community gets the benefit as well without the need for other tools.

However we would definitely pay at least a partial bounty to an interim solution that is client integrated, but not necessarily this complete set. Such as pull based equivalents, or the ability to check arbitrary addresses for payments.

 
kjj
legendary
Activity: 1302
Merit: 1026
We need these features for our applications, so were are prepared to put our money where are mouth is.  We will pay a 25btc bounty for the integration of these features.  To be split between the key participants in making this patch a reality.

This thread covers several different things.  Exactly which features are you offering the bounty for?
newbie
Activity: 31
Merit: 0
We need these features for our applications, so were are prepared to put our money where are mouth is.  We will pay a 25btc bounty for the integration of these features.  To be split between the key participants in making this patch a reality.

member
Activity: 70
Merit: 18
Seems the natural solution would to be to have an "address watch list" that can maintain balances at addresses without the private key anywhere in sight.  I would have expected the wallet to use something along those lines internally anyway, but not sure how hard it would be to expose that functionality with arbitrary addresses and no private keys.
hero member
Activity: 588
Merit: 500
If I understand correctly this patch will allow web application servers running bticoind to confirm payments without an active receiving wallet.

I would like to reiterate that this is an important feature. Since the mybitcoin debacle there is now a deadlock for merchant services between the risk of a server side wallet, and the risk of 3rd party merchant services. I am shocked to find out that getreceivedbyaddress does not give a correct value for any address except those in your wallet. This means that a receiving wallet must be left on the server and it will receive ALL bitcoin income for the service. Even with code to regularly sweep the account, this is clearly an unacceptable security risk.  On the other hand all 3rd party solutions are even riskier.

Conscientious merchants simply can't move forward with bitcoin without this patch.

j

This is part of the reason I stopped using this patch myself. At the moment I am trying to stabilize blkmond, which provides a realtime view of the whole Bitcoin network, including all transaction activity, but likes to stop running unexpectedly. It can notify of payments received at an address without access to the wallet containing the corresponding key, but not if it's crashed.
full member
Activity: 175
Merit: 102
I took error's work and further tweaked so it works (and is rebased against) latest git head.

... but I'm not 100% happy with it. I'm not sure it properly handles block chain re-orgs and dependent orphan transactions. Would be nice to write some tests to exercise those edge cases, and figure out what it SHOULD do in those cases.


Gavin,

Yes, I initially included this patch in my build of bitcoind but decided not to use it for exactly the reasons you posit. I realized it would lead to apparent block confirmations that ended up being orphaned blocks, and/or reporting transactions that would not ultimately be confirmed.  In other words - both monitors can lead to a false sense of security.

Sigh Sad Unfortunately I don't have the time to exercise the edge cases. 

I may in the future, offer a bounty to someone who does exercise those cases, and writes the appropriate code to deal with them.
member
Activity: 76
Merit: 12
If I understand correctly this patch will allow web application servers running bticoind to confirm payments without an active receiving wallet.

I would like to reiterate that this is an important feature. Since the mybitcoin debacle there is now a deadlock for merchant services between the risk of a server side wallet, and the risk of 3rd party merchant services. I am shocked to find out that getreceivedbyaddress does not give a correct value for any address except those in your wallet. This means that a receiving wallet must be left on the server and it will receive ALL bitcoin income for the service. Even with code to regularly sweep the account, this is clearly an unacceptable security risk.  On the other hand all 3rd party solutions are even riskier.

Conscientious merchants simply can't move forward with bitcoin without this patch.

j
legendary
Activity: 1652
Merit: 2301
Chief Scientist
I took error's work and further tweaked so it works (and is rebased against) latest git head.

... but I'm not 100% happy with it. I'm not sure it properly handles block chain re-orgs and dependent orphan transactions. Would be nice to write some tests to exercise those edge cases, and figure out what it SHOULD do in those cases.
hero member
Activity: 588
Merit: 500
This needed a lot of hackery to apply to current git. You can find my patch here. I would still clean it up further before putting it in Bitcoin since I basically just moved a whole class into rpc.h and that's hardly elegant.

http://dl.dropbox.com/u/24777749/0001-New-RPC-calls-monitortx-monitorblocks-listmonitored.patch
newbie
Activity: 22
Merit: 0
I actually got this code compiled under g++, pulled it into the 3.22 codebase, and made my edits from there.

The git diff is available here:

http://pastebin.com/wwtDCErJ


I'm still getting some problems getting this pull request to compile against master, even after applying the diff. Could you possibly publish your git branch?
newbie
Activity: 22
Merit: 0
This would be incredibly useful for me, and would make a lot of work I was planning on doing around polling unnecessary.
hero member
Activity: 540
Merit: 500
"monitortx" could be really usefull for merchants who wait for payments to process orders. This works like a bank call to confirm a credit card transaction.

Quote
I presume monitortx is sending info on transactions received but not yet incorporated into a block?
If this is the case, a call to "gettransaction" will be needed to check the desired number of confirmations :p
newbie
Activity: 2
Merit: 0
I actually got this code compiled under g++, pulled it into the 3.22 codebase, and made my edits from there.

The git diff is available here:

http://pastebin.com/wwtDCErJ
newbie
Activity: 2
Merit: 0
Yes this looks like a great feature, yay for pushing opposed to polling.
Pages:
Jump to: