Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow clients to cancel transaction notifications #342

Merged
merged 1 commit into from
May 5, 2015

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Mar 5, 2015

This PR aims to address #122 by providing JSON-RPC methods allowing clients to cancel received and spent notifications.

Note: before this can be properly integrated into btcwallet, btcrpcclient will need to be updated to use btcjsonv2.

@Roasbeef Roasbeef force-pushed the stopnotify branch 3 times, most recently from b9b72f4 to 22aea26 Compare March 5, 2015 22:14
@davecgh
Copy link
Member

davecgh commented Mar 10, 2015

Thanks. I'll review this within the next few days.

}

// StopNotifyReceivedCmd defines the stopnotifyreceived JSON-RPC command.
type StopNotifyReceivedCmd struct {
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpick, but all of the other command definitions alphabetized, so I think this should be defined before type StopNotifySpentCmd struct to be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@davecgh
Copy link
Member

davecgh commented Mar 11, 2015

I need to test this out still, but aside from my minor nitpick, this reads well.

@davecgh
Copy link
Member

davecgh commented Apr 1, 2015

I noticed this includes stopnotifyspent and stopnotifyreceived, but it lacks stop for the other 2 notification registrations. In particular, stopnotifyblocks and stopnotifynewtransactions.

@Roasbeef Roasbeef force-pushed the stopnotify branch 2 times, most recently from ef0b70b to dcdc991 Compare April 6, 2015 17:54
@davecgh
Copy link
Member

davecgh commented Apr 13, 2015

I've tested this and everything looks good. The code reads well too. Just need to squash and rebase it and it's good to go.

OK

@davecgh
Copy link
Member

davecgh commented Apr 13, 2015

Oh, I just realized I was a bit hasty. While the code is correct and works properly, I noticed the documentation for the JSON-RPC API wasn't updatd. The new commands need to be added to the websocket extensions section of docs/json_rpc_api.md.

@davecgh
Copy link
Member

davecgh commented Apr 21, 2015

This is just waiting on the documentation updates.

@Roasbeef
Copy link
Member Author

The documentation has been updated with the latest commit.

@davecgh
Copy link
Member

davecgh commented May 1, 2015

Thanks for the udpates. OK after rebase and squash.

@Roasbeef
Copy link
Member Author

Roasbeef commented May 5, 2015

Rebased and squashed 👍

@davecgh
Copy link
Member

davecgh commented May 5, 2015

Thanks, but it's not against the latest master!

This commit adds 4 new websockets JSON-RPC methods for canceling
notifications:
  * stopnotifyspent
  * stopnotifyreceived
  * stopnotifyblocks
  * stopnotifynewtransactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants