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.

Sign up for GitHub

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

Prevent unnecessary wallet rescans. #519

Merged
merged 2 commits into from
Sep 13, 2019
Merged

Prevent unnecessary wallet rescans. #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c106e3
Prevent unnecessary wallet rescans.
jholdstock Sep 10, 2019
e96ed20
Joe review
jholdstock Sep 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions backend/stakepoold/rpc/api.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ service StakepooldService {
rpc GetLiveTickets (GetLiveTicketsRequest) returns (GetLiveTicketsResponse);
rpc SetAddedLowFeeTickets (SetAddedLowFeeTicketsRequest) returns (SetAddedLowFeeTicketsResponse);
rpc SetUserVotingPrefs (SetUserVotingPrefsRequest) returns (SetUserVotingPrefsResponse);
rpc ImportScript (ImportScriptRequest) returns (ImportScriptResponse);
rpc ImportNewScript (ImportNewScriptRequest) returns (ImportNewScriptResponse);
rpc ImportMissingScripts (ImportMissingScriptsRequest) returns (ImportMissingScriptsResponse);
rpc StakePoolUserInfo (StakePoolUserInfoRequest) returns (StakePoolUserInfoResponse);
rpc WalletInfo (WalletInfoRequest) returns (WalletInfoResponse);
rpc ValidateAddress (ValidateAddressRequest) returns (ValidateAddressResponse);
Expand Down Expand Up @@ -78,12 +79,17 @@ message AccountSyncAddressIndexRequest {
message AccountSyncAddressIndexResponse {
}

message ImportScriptRequest {
message ImportMissingScriptsRequest {
repeated bytes Scripts = 1;
int64 RescanHeight = 2;
}
message ImportMissingScriptsResponse {
}

message ImportNewScriptRequest {
bytes Script = 1;
bool Rescan = 2;
int64 RescanHeight = 3;
}
message ImportScriptResponse {
message ImportNewScriptResponse {
int64 HeightImported = 1;
}

Expand Down
46 changes: 40 additions & 6 deletions backend/stakepoold/rpc/rpcserver/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,19 +277,53 @@ func (ctx *AppContext) UpdateTicketDataFromMySQL() error {
return nil
}

func (ctx *AppContext) ImportScript(script []byte, rescan bool, rescanHeight int64) (int64, error) {
err := ctx.WalletConnection.ImportScriptRescanFrom(script, rescan, int(rescanHeight))
// ImportNewScript will import a redeem script into dcrwallet. No rescan is
// performed because we are importing a brand new script, it shouldn't have any
// associated history. Current block height is returned to indicate which height
// the new user has registered.
func (ctx *AppContext) ImportNewScript(script []byte) (int64, error) {
jholdstock marked this conversation as resolved.
Show resolved Hide resolved

err := ctx.WalletConnection.ImportScriptRescanFrom(script, false, 0)
if err != nil {
log.Errorf("ImportScript: ImportScript rpc failed: %v", err)
log.Errorf("ImportNewScript: ImportScriptRescanFrom rpc failed: %v", err)
return -1, err
}

_, block, err := ctx.WalletConnection.GetBestBlock()
_, bestBlockHeight, err := ctx.WalletConnection.GetBestBlock()
if err != nil {
log.Errorf("ImportScript: getBetBlock rpc failed: %v", err)
log.Errorf("ImportNewScript: GetBestBlock rpc failed: %v", err)
return -1, err
}
return block, nil
return bestBlockHeight, nil
}

// ImportMissingScripts accepts a list of redeem scripts and a rescan height. It
// will import all but one of the scripts without triggering a wallet rescan,
// and finally trigger a rescan from the provided height after importing the
// last one.
func (ctx *AppContext) ImportMissingScripts(scripts [][]byte, rescanHeight int) error {
jholdstock marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Member

Choose a reason for hiding this comment

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

Do you think it's worth checking that len(scripts) != 0 since it could cause a panic? I do realize that you are checking before calling, but someone else may not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, we don't currently have any request validation for the stakepoold RPC server, and I would be reluctant to add it for only one request. We should either add validation to all requests for consistency, or seeing as are controlling both the client and the server code, and seeing as we dont expect any additional clients, I think we could leave it as-is. Obviously we would be a bit more careful if this were a public API with external clients.

Happy to change if others disagree.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. Maybe just a comment then?

// Import n-1 scripts without a rescan.
allButOne := scripts[:len(scripts)-1]
for _, script := range allButOne {
err := ctx.WalletConnection.ImportScriptRescanFrom(script, false, 0)
if err != nil {
log.Errorf("ImportMissingScripts: ImportScript rpc failed: %v", err)
return err
}
}

// Import the last script and trigger a rescan
lastOne := scripts[len(scripts)-1]
err := ctx.WalletConnection.ImportScriptRescanFrom(lastOne, true, rescanHeight)
if err != nil {
log.Errorf("ImportMissingScripts: ImportScriptRescanFrom rpc failed: %v", err)
return err
}

log.Infof("ImportMissingScripts: Imported %d scripts and triggered a rescan from height %d", len(scripts), rescanHeight)

return nil
}

func (ctx *AppContext) AddMissingTicket(ticketHash []byte) error {
Expand Down
22 changes: 17 additions & 5 deletions backend/stakepoold/rpc/rpcserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const (
// collection cycle to also trigger a timeout but the current allocation
// pattern of stakepoold is not known to cause such conditions at this time.
GRPCCommandTimeout = time.Millisecond * 100
semverString = "7.0.0"
semverMajor = 7
semverString = "8.0.0"
semverMajor = 8
semverMinor = 0
semverPatch = 0
)
Expand Down Expand Up @@ -184,20 +184,32 @@ func (s *stakepooldServer) SetUserVotingPrefs(ctx context.Context, req *pb.SetUs
return &pb.SetUserVotingPrefsResponse{}, nil
}

func (s *stakepooldServer) ImportScript(ctx context.Context, req *pb.ImportScriptRequest) (*pb.ImportScriptResponse, error) {
func (s *stakepooldServer) ImportNewScript(ctx context.Context, req *pb.ImportNewScriptRequest) (*pb.ImportNewScriptResponse, error) {
if !s.walletConnected() {
return nil, ErrWalletNotConnected
}

heightImported, err := s.appContext.ImportScript(req.Script, req.Rescan, req.RescanHeight)
heightImported, err := s.appContext.ImportNewScript(req.Script)
if err != nil {
return nil, err
}
return &pb.ImportScriptResponse{
return &pb.ImportNewScriptResponse{
HeightImported: heightImported,
}, nil
}

func (s *stakepooldServer) ImportMissingScripts(ctx context.Context, req *pb.ImportMissingScriptsRequest) (*pb.ImportMissingScriptsResponse, error) {
if !s.walletConnected() {
return nil, ErrWalletNotConnected
}

err := s.appContext.ImportMissingScripts(req.Scripts, int(req.RescanHeight))
if err != nil {
return nil, err
}
return &pb.ImportMissingScriptsResponse{}, nil
}

func (s *stakepooldServer) ListScripts(ctx context.Context, req *pb.ListScriptsRequest) (*pb.ListScriptsResponse, error) {
if !s.walletConnected() {
return nil, ErrWalletNotConnected
Expand Down
Loading