From dcf2735836ba5993f9a876139ff8476fc9c18a8e Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Tue, 22 Sep 2020 20:19:31 -0300 Subject: [PATCH] move validation from protocol to API --- chain/exchange/client.go | 45 +++++++++++++++++++++++++++----------- chain/exchange/protocol.go | 15 +++++-------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/chain/exchange/client.go b/chain/exchange/client.go index a99362745..371c50aed 100644 --- a/chain/exchange/client.go +++ b/chain/exchange/client.go @@ -65,7 +65,15 @@ func NewClient(lc fx.Lifecycle, host host.Host, pmgr peermgr.MaybePeerMgr) Clien // request options without disrupting external calls. In the future the // consumers should be forced to use a more standardized service and // adhere to a single API derived from this function. -func (c *client) doRequest(ctx context.Context, req *Request, singlePeer *peer.ID) (*validatedResponse, error) { +func (c *client) doRequest( + ctx context.Context, + req *Request, + singlePeer *peer.ID, + // In the `GetChainMessages` case, we won't request the headers but we still + // need them to check the integrity of the `CompactedMessages` in the response + // so the tipset blocks need to be provided by the caller. + tipsets []*types.TipSet, +) (*validatedResponse, error) { // Validate request. if req.Length == 0 { return nil, xerrors.Errorf("invalid request of length 0") @@ -116,7 +124,7 @@ func (c *client) doRequest(ctx context.Context, req *Request, singlePeer *peer.I } // Process and validate response. - validRes, err := c.processResponse(req, res) + validRes, err := c.processResponse(req, res, tipsets) if err != nil { log.Warnf("processing peer %s response failed: %s", peer.String(), err) @@ -144,7 +152,7 @@ func (c *client) doRequest(ctx context.Context, req *Request, singlePeer *peer.I // errors. Peer penalization should happen here then, before returning, so // we can apply the correct penalties depending on the cause of the error. // FIXME: Add the `peer` as argument once we implement penalties. -func (c *client) processResponse(req *Request, res *Response) (*validatedResponse, error) { +func (c *client) processResponse(req *Request, res *Response, tipsets []*types.TipSet) (*validatedResponse, error) { err := res.statusToError() if err != nil { return nil, xerrors.Errorf("status error: %s", err) @@ -176,6 +184,16 @@ func (c *client) processResponse(req *Request, res *Response) (*validatedRespons // Check for valid block sets and extract them into `TipSet`s. validRes.tipsets = make([]*types.TipSet, resLength) for i := 0; i < resLength; i++ { + if res.Chain[i] == nil { + return nil, xerrors.Errorf("response with nil tipset in pos %d", i) + } + for blockIdx, block := range res.Chain[i].Blocks { + if block == nil { + return nil, xerrors.Errorf("tipset with nil block in pos %d", blockIdx) + // FIXME: Maybe we should move this check to `NewTipSet`. + } + } + validRes.tipsets[i], err = types.NewTipSet(res.Chain[i].Blocks) if err != nil { return nil, xerrors.Errorf("invalid tipset blocks at height (head - %d): %w", i, err) @@ -214,14 +232,16 @@ func (c *client) processResponse(req *Request, res *Response) (*validatedRespons if err != nil { return nil, err } - } - - if options.ValidateMessages { - // if the request includes target tipsets, validate against them + } else { + // If we didn't request the headers they should have been provided + // by the caller. + if len(tipsets) < len(res.Chain) { + return nil, xerrors.Errorf("not enought tipsets provided for message response validation, needed %d, have %d", len(res.Chain), len(tipsets)) + } chain := make([]*BSTipSet, 0, resLength) for i, resChain := range res.Chain { next := &BSTipSet{ - Blocks: req.TipSets[i].Blocks(), + Blocks: tipsets[i].Blocks(), Messages: resChain.Messages, } chain = append(chain, next) @@ -290,7 +310,7 @@ func (c *client) GetBlocks(ctx context.Context, tsk types.TipSetKey, count int) Options: Headers, } - validRes, err := c.doRequest(ctx, req, nil) + validRes, err := c.doRequest(ctx, req, nil, nil) if err != nil { return nil, err } @@ -308,7 +328,7 @@ func (c *client) GetFullTipSet(ctx context.Context, peer peer.ID, tsk types.TipS Options: Headers | Messages, } - validRes, err := c.doRequest(ctx, req, &peer) + validRes, err := c.doRequest(ctx, req, &peer, nil) if err != nil { return nil, err } @@ -335,11 +355,10 @@ func (c *client) GetChainMessages(ctx context.Context, tipsets []*types.TipSet) req := &Request{ Head: head.Cids(), Length: length, - Options: Messages | Validate, - TipSets: tipsets, + Options: Messages, } - validRes, err := c.doRequest(ctx, req, nil) + validRes, err := c.doRequest(ctx, req, nil, tipsets) if err != nil { return nil, err } diff --git a/chain/exchange/protocol.go b/chain/exchange/protocol.go index b1fe69bd2..211479335 100644 --- a/chain/exchange/protocol.go +++ b/chain/exchange/protocol.go @@ -57,8 +57,6 @@ type Request struct { // Request options, see `Options` type for more details. Compressed // in a single `uint64` to save space. Options uint64 - // Request tipsets for validation - TipSets []*types.TipSet } // `Request` processed and validated to query the tipsets needed. @@ -73,15 +71,13 @@ type validatedRequest struct { const ( Headers = 1 << iota Messages - Validate ) // Decompressed options into separate struct members for easy access // during internal processing.. type parsedOptions struct { - IncludeHeaders bool - IncludeMessages bool - ValidateMessages bool + IncludeHeaders bool + IncludeMessages bool } func (options *parsedOptions) noOptionsSet() bool { @@ -91,9 +87,8 @@ func (options *parsedOptions) noOptionsSet() bool { func parseOptions(optfield uint64) *parsedOptions { return &parsedOptions{ - IncludeHeaders: optfield&(uint64(Headers)) != 0, - IncludeMessages: optfield&(uint64(Messages)) != 0, - ValidateMessages: optfield&(uint64(Validate)) != 0, + IncludeHeaders: optfield&(uint64(Headers)) != 0, + IncludeMessages: optfield&(uint64(Messages)) != 0, } } @@ -144,6 +139,8 @@ func (res *Response) statusToError() error { // FIXME: Rename. type BSTipSet struct { + // List of blocks belonging to a single tipset to which the + // `CompactedMessages` are linked. Blocks []*types.BlockHeader Messages *CompactedMessages }