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

Add new stake-related RPCs to the gRPC interface #293

Merged
merged 1 commit into from
Jul 20, 2016

Conversation

cjepson
Copy link
Contributor

@cjepson cjepson commented Jul 11, 2016

The following new stake related RPC calls have been added to the
gRPC:

  1. StakeInfo
  2. TicketPrice
  3. ImportScript
  4. PurchaseTickets

ImportPrivateKey has been updated to take the new ScanFrom variable
from Decred.

The GetStakeInfo handling method for the legacy RPC has been modified
to call a more general API from wallet.

The CreatePurchaseTicket API in wallet has been modified to take the
fees to use for both the split transaction and ticket as arguments.

The SemVer minor version for the gRPC server has been bumped to 2.2.0.

The function to retrieve the stake difficulty from wallet has been
changed to simply use the fetch function from the daemon, and the
registration for stake difficulty notifications has been removed.

@chappjc
Copy link
Member

chappjc commented Jul 11, 2016

So this is happening. Will all legacy RPC commands get translated?
I'll start trying gRPC in dcrspy I suppose.

@cjepson
Copy link
Contributor Author

cjepson commented Jul 11, 2016

@chappjc It's for the GUI, but in the future most new RPC calls will probably go through the gRPC. I doubt that all old commands will be translated, just the most important ones.

@@ -277,14 +279,42 @@ func (s *walletServer) ImportPrivateKey(ctx context.Context, req *pb.ImportPriva
"Only the imported account accepts private key imports")
}

_, err = s.wallet.ImportPrivateKey(wif, nil, req.Rescan, 0)
_, err = s.wallet.ImportPrivateKey(wif, nil, req.Rescan, req.ScanFrom)
Copy link
Member

Choose a reason for hiding this comment

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

Before this, I would check for non-zero scan_from and error when rescan is false.

@alexlyp
Copy link
Member

alexlyp commented Jul 11, 2016

seems to run fine with ticketbuyer, tACK for legacy usage

@jrick
Copy link
Member

jrick commented Jul 11, 2016

Mentioned in chat but saying it here too: needs doco added to rpc/documentation/api.md for all of the additions.

#### `ImportPrivateKey`

The `ImportScript` method imports a script into the transaction store and address
manager of the wallet.. A rescan may optionally be started to search for
Copy link
Member

Choose a reason for hiding this comment

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

Extra period.

@jrick
Copy link
Member

jrick commented Jul 12, 2016

See https://godoc.org/google.golang.org/grpc/codes for the meanings of all of the gRPC status codes.


___

#### `ImportPrivateKey`
Copy link
Member

Choose a reason for hiding this comment

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

Wrong header

@jcvernaleo
Copy link
Member

I did some testing for normal wallet stuff (send, recieve, getstakeinfo, purchaseticket, etc) and all are fine. So while I have no comment on the grpc stuff (all you @jrick ) this doesn't break anything.

uint32 live = 5;
uint32 voted = 6;
uint32 missed = 7;
uint32 revoked = 8;
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a placeholder for expired tickets?
https://forum.decred.org/threads/one-of-my-oldest-tickets-rip.3873/#post-21184

Copy link
Member

Choose a reason for hiding this comment

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

Additional fields can be added in a backward-compatible way.

However i do think we should get it right the first time where possible. @cjepson can you add this field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add it, it's entirely possible in the future that it might exist. For now the server can simply set it to 0 and it can be ignored at the UI level.

@cjepson cjepson force-pushed the cj_080716grpcpurchaseticket branch from fd0ad0b to 46ecc53 Compare July 14, 2016 01:19
optionally be started to search for transactions involving the script, either
as an output or in a P2SH input.

**Request:** `ImportScriptRequest`
Copy link
Member

Choose a reason for hiding this comment

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

This message doco is missing the scan_from field.

Copy link
Member

Choose a reason for hiding this comment

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

Also just noticed that this is missing the account. We should add it to the message and error whenever a script is imported into anything other than the imported account. Later on we will enable both script and private key imports for other accounts as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add scan_from, but I don't think we should have accounts for scripts as scripts can span no accounts (OP_TRUE) or many accounts (2-of-2 P2SH involving 2 accounts from the same wallet). That's why wallet also puts them in txstore unencrypted now, and in the future I would consider stripping them from the address manager completely because they're fundamentally different from an account or private key.

Copy link
Member

Choose a reason for hiding this comment

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

Noted, that makes sense.

@cjepson cjepson force-pushed the cj_080716grpcpurchaseticket branch 3 times, most recently from 0dd7191 to d37bab1 Compare July 19, 2016 22:45
@jrick
Copy link
Member

jrick commented Jul 19, 2016

The PurchaseTicket doco needs to cover the passphrase parameter that was just added and another expected error for an invalid passphrase.

@cjepson cjepson force-pushed the cj_080716grpcpurchaseticket branch 2 times, most recently from e488487 to aa4624e Compare July 20, 2016 00:11
current stake difficulty is above this amount, the wallet will return an error.

- `uint32 required_confirmations`: The number of required confirmations for
funds used to purchase a ticket. If left unset, it will use unconfirmed and
Copy link
Member

Choose a reason for hiding this comment

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

if left unset -> if zero

@alexlyp
Copy link
Member

alexlyp commented Jul 20, 2016

tACK with provided grpc example

The following new stake related RPC calls have been added to the
gRPC:

 1. StakeInfo
 2. TicketPrice
 3. ImportScript
 4. PurchaseTickets

ImportPrivateKey has been updated to take the new ScanFrom variable
from Decred.

The GetStakeInfo handling method for the legacy RPC has been modified
to call a more general API from wallet.

The CreatePurchaseTicket API in wallet has been modified to take the
fees to use for both the split transaction and ticket as arguments.

The SemVer minor version for the gRPC server has been bumped to 2.2.0.

The function to retrieve the stake difficulty from wallet has been
changed to simply use the fetch function from the daemon, and the
registration for stake difficulty notifications has been removed.
@cjepson cjepson force-pushed the cj_080716grpcpurchaseticket branch from aa4624e to 6087ab2 Compare July 20, 2016 16:24
@alexlyp alexlyp merged commit fa20b91 into decred:master Jul 20, 2016
@jcvernaleo jcvernaleo added this to the v0.2.0 milestone Jul 22, 2016
beansgum pushed a commit to beansgum/dcrwallet that referenced this pull request Jun 10, 2021
* Adds Privacy Page layout

* Update Privacy Page
- add wallet menu privacy item
- create subpage layout for settings page
- add resuable headers for subpage layouts
- implement dangerZoneLayout collapsible
- create privacyInfo modal
- add modifiable title for modal
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.

5 participants