From ed81fb54ec15328be9bc21912face5ba5c55c5e2 Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Fri, 5 Jan 2018 13:24:16 +0100 Subject: [PATCH 1/3] NewInquiring returns error instead of swallowing it --- lite/inquirer.go | 12 ++++++++---- lite/inquirer_test.go | 9 +++++---- lite/proxy/certifier.go | 7 ++++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lite/inquirer.go b/lite/inquirer.go index 5d6ce60c7..9d0d77e51 100644 --- a/lite/inquirer.go +++ b/lite/inquirer.go @@ -23,16 +23,20 @@ type Inquiring struct { // // Example: The trusted provider should a CacheProvider, MemProvider or files.Provider. The source // provider should be a client.HTTPProvider. -func NewInquiring(chainID string, fc FullCommit, trusted Provider, source Provider) *Inquiring { +func NewInquiring(chainID string, fc FullCommit, trusted Provider, + source Provider) (*Inquiring, error) { + // store the data in trusted - // TODO: StoredCommit() can return an error and we need to handle this. - trusted.StoreCommit(fc) + err := trusted.StoreCommit(fc) + if err != nil { + return nil, err + } return &Inquiring{ cert: NewDynamic(chainID, fc.Validators, fc.Height()), trusted: trusted, Source: source, - } + }, nil } // ChainID returns the chain id. diff --git a/lite/inquirer_test.go b/lite/inquirer_test.go index ce4317543..97ad60e3a 100644 --- a/lite/inquirer_test.go +++ b/lite/inquirer_test.go @@ -36,7 +36,7 @@ func TestInquirerValidPath(t *testing.T) { } // initialize a certifier with the initial state - cert := lite.NewInquiring(chainID, commits[0], trust, source) + cert, _ := lite.NewInquiring(chainID, commits[0], trust, source) // this should fail validation.... commit := commits[count-1].Commit @@ -85,7 +85,7 @@ func TestInquirerMinimalPath(t *testing.T) { } // initialize a certifier with the initial state - cert := lite.NewInquiring(chainID, commits[0], trust, source) + cert, _ := lite.NewInquiring(chainID, commits[0], trust, source) // this should fail validation.... commit := commits[count-1].Commit @@ -130,11 +130,12 @@ func TestInquirerVerifyHistorical(t *testing.T) { h := int64(20 + 10*i) appHash := []byte(fmt.Sprintf("h=%d", h)) resHash := []byte(fmt.Sprintf("res=%d", h)) - commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, len(keys)) + commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, + len(keys)) } // initialize a certifier with the initial state - cert := lite.NewInquiring(chainID, commits[0], trust, source) + cert, _ := lite.NewInquiring(chainID, commits[0], trust, source) // store a few commits as trust for _, i := range []int{2, 5} { diff --git a/lite/proxy/certifier.go b/lite/proxy/certifier.go index 1d7284f2c..3dda935ea 100644 --- a/lite/proxy/certifier.go +++ b/lite/proxy/certifier.go @@ -25,6 +25,11 @@ func GetCertifier(chainID, rootDir, nodeAddr string) (*lite.Inquiring, error) { if err != nil { return nil, err } - cert := lite.NewInquiring(chainID, fc, trust, source) + + cert, err := lite.NewInquiring(chainID, fc, trust, source) + if err != nil { + return nil, err + } + return cert, nil } From ba475d312819078d35695467f22bbf45585e6e75 Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Fri, 5 Jan 2018 13:25:58 +0100 Subject: [PATCH 2/3] Fix formatting --- lite/inquirer_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lite/inquirer_test.go b/lite/inquirer_test.go index 97ad60e3a..510059674 100644 --- a/lite/inquirer_test.go +++ b/lite/inquirer_test.go @@ -32,7 +32,8 @@ func TestInquirerValidPath(t *testing.T) { vals := keys.ToValidators(vote, 0) h := int64(20 + 10*i) appHash := []byte(fmt.Sprintf("h=%d", h)) - commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, len(keys)) + commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, + len(keys)) } // initialize a certifier with the initial state @@ -81,7 +82,8 @@ func TestInquirerMinimalPath(t *testing.T) { h := int64(5 + 10*i) appHash := []byte(fmt.Sprintf("h=%d", h)) resHash := []byte(fmt.Sprintf("res=%d", h)) - commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, len(keys)) + commits[i] = keys.GenFullCommit(chainID, h, nil, vals, appHash, consHash, resHash, 0, + len(keys)) } // initialize a certifier with the initial state From 13fa23c56854a855631b22a1149780fe64ec5760 Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Sat, 6 Jan 2018 22:24:58 +0100 Subject: [PATCH 3/3] Add error checking --- lite/inquirer_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lite/inquirer_test.go b/lite/inquirer_test.go index 510059674..25bf51c4f 100644 --- a/lite/inquirer_test.go +++ b/lite/inquirer_test.go @@ -37,14 +37,15 @@ func TestInquirerValidPath(t *testing.T) { } // initialize a certifier with the initial state - cert, _ := lite.NewInquiring(chainID, commits[0], trust, source) + cert, err := lite.NewInquiring(chainID, commits[0], trust, source) + require.Nil(err) // this should fail validation.... commit := commits[count-1].Commit - err := cert.Certify(commit) + err = cert.Certify(commit) require.NotNil(err) - // add a few seed in the middle should be insufficient + // adding a few commits in the middle should be insufficient for i := 10; i < 13; i++ { err := source.StoreCommit(commits[i]) require.Nil(err)