-
Notifications
You must be signed in to change notification settings - Fork 21
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
implement peer snapshot client and associated methods #212
base: main
Are you sure you want to change the base?
Changes from 1 commit
6304ddf
d729e66
c1a0012
d746ad9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
package snapshot | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/hyperledger/fabric-admin-sdk/internal/protoutil" | ||
"github.com/hyperledger/fabric-admin-sdk/pkg/identity" | ||
"github.com/hyperledger/fabric-protos-go-apiv2/common" | ||
"github.com/hyperledger/fabric-protos-go-apiv2/peer" | ||
"google.golang.org/grpc" | ||
"google.golang.org/protobuf/proto" | ||
) | ||
|
||
// Client is a wrapper around snapshot client | ||
type Client struct { | ||
snapshot peer.SnapshotClient | ||
id identity.SigningIdentity | ||
} | ||
|
||
func New(conn grpc.ClientConnInterface, id identity.SigningIdentity) *Client { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be perfectly named if it was creating a Snapshot, since the package is called snapshot. It might be clearer (and more consistent with other packages in this module) if it names what it is creating. For example, NewPeer. |
||
return &Client{ | ||
snapshot: peer.NewSnapshotClient(conn), | ||
id: id, | ||
} | ||
} | ||
|
||
// SubmitSnapshotRequest from a specific peer | ||
func (s *Client) SubmitSnapshotRequest(ctx context.Context, channelID string, blockNum uint64) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are in a nicely named snapshot package, perhaps this can be shortened to just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree it's clearer if we remove the Snapshot portion from the client methods. |
||
signedRequest, err := s.newSignedSnapshotRequest(channelID, blockNum) | ||
if err != nil { | ||
return fmt.Errorf("failed to create signed snapshot request: %w", err) | ||
} | ||
|
||
if _, err := s.snapshot.Generate(ctx, signedRequest); err != nil { | ||
return fmt.Errorf("failed to submit generate snapshot request: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// CancelSnapshotRequest from a specific peer | ||
func (s *Client) CancelSnapshotRequest(ctx context.Context, channelID string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this can be called just |
||
signedRequest, err := s.newSignedSnapshotRequest(channelID, 0) | ||
if err != nil { | ||
return fmt.Errorf("failed to create signed snapshot request: %w", err) | ||
} | ||
|
||
if _, err := s.snapshot.Cancel(ctx, signedRequest); err != nil { | ||
return fmt.Errorf("failed to cancel snapshot request: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// ListPendingSnapshots from a specific peer | ||
func (s *Client) ListPendingSnapshots(ctx context.Context, channelID string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can lose the The big problem with this method, which needs to be fixed, is that it queries information but returns nothing. I would return a QueryPendingSnapshotsResponse object rather than the slice of block numbers that it contains, in case that protobuf message gets extended in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out. I was copying and pasting the outlying structure of the methods and missed it. My bad. |
||
signedRequest, err := s.newSignedSnapshotRequest(channelID, 0) | ||
if err != nil { | ||
return fmt.Errorf("failed to create signed snapshot request: %w", err) | ||
} | ||
|
||
if _, err := s.snapshot.QueryPendings(ctx, signedRequest); err != nil { | ||
return fmt.Errorf("failed to list pending snapshots: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// newSignedSnapshotRequest returns a signed snapshot request for | ||
// given channel | ||
func (s *Client) newSignedSnapshotRequest(channelID string, blockNum uint64) (*peer.SignedSnapshotRequest, error) { | ||
nonce, err := protoutil.CreateNonce() | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to create nonce: %w", err) | ||
} | ||
|
||
request, err := proto.Marshal(&peer.SnapshotRequest{ | ||
SignatureHeader: &common.SignatureHeader{ | ||
Creator: s.id.Credentials(), | ||
Nonce: nonce, | ||
}, | ||
ChannelId: channelID, | ||
BlockNumber: blockNum, | ||
}) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to marshal snapshot request: %w", err) | ||
} | ||
|
||
signature, err := s.id.Sign(request) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to sign snapshot request: %w", err) | ||
} | ||
|
||
return &peer.SignedSnapshotRequest{ | ||
Request: request, | ||
Signature: signature, | ||
}, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the chaincode and discovery packages we use Peer to make it clear to the user which component of the Fabric network the are interacting with, and to which they need to supply a client connection. I think this might be better named as Peer for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I even started writing with it, but only from reading the codebase I wasn't able to understand the naming, but it makes sense to me now. Renamed it to Peer.