-
Notifications
You must be signed in to change notification settings - Fork 573
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
Add wallet/setAccountHead RPC #4951
Conversation
3c8f732
to
29b54d0
Compare
e532a4f
to
540e75a
Compare
540e75a
to
152bf87
Compare
This PR looks extremely well written to me. The tests are very easy to verify. My only comment here would be, we should abort eagerly when we can if the request is closed so we don't continue using up CPU if the client disconnects. Especially because we don't have an account lock. It'll prevent the client re-trying the request and causing havoc if they disconnect. |
I added early-exit to both of the loops. Tested it by curling |
Summary
Implements an RPC for manually updating an account's head. The user can pass in hashes of transactions known to be owned by the account, and the wallet will decrypt and apply only those transactions to the account's state.
Needs Review
Should the name of the RPC route be different?Renamed from wallet/addKnownTransactions to wallet/setAccountHeadShould this logic live somewhere other than an RPC route, e.g. integrated with the wallet?We might move this out into the Scanner once it's refactored out of the wallet.Should the route have wallet context instead of fullnode?Decided this is fine for now since it's similar to a service command anyway.What should we do if someone cancels an RPC request before updateHead finishes?Added early-exit to the loops by checkingrequest.closed
.Testing Plan
Added automated tests for the RPC.
Manual testing
wallet:scanning:off
to disable scanning for an accountwallet:transactions
on the account. Save all transaction hashes.wallet:status
. Save the account's head hash asend
wallet:rescan
to reset the account (Add wallet/resetAccount RPC #4985 will allow you to do this for one account). Cancel the scanwallet:status
again. Save the account's head hash asstart
start
on the directory.repl
on the directory. Execute the command with the values you previously saved.After running
wallet:status
again, you should see the account restored to its former state.Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
iron-fish/website#706
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and label it with
breaking-change-rpc
orbreaking-change-sdk
.