From 2cd8fa7e0a67e86415b35e446604763fd4b5b275 Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 2 Oct 2020 07:02:53 -0500 Subject: [PATCH 1/6] Use snake_case for claim predicate JSON fields. (#3086) --- services/horizon/internal/resourceadapter/operations_test.go | 4 ++-- xdr/json.go | 4 ++-- xdr/json_test.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/horizon/internal/resourceadapter/operations_test.go b/services/horizon/internal/resourceadapter/operations_test.go index 2237754f88..b85d0a7d18 100644 --- a/services/horizon/internal/resourceadapter/operations_test.go +++ b/services/horizon/internal/resourceadapter/operations_test.go @@ -143,8 +143,8 @@ func TestPopulateOperation_CreateClaimableBalance(t *testing.T) { "and": [ { "or": [ - {"relBefore":"12"}, - {"absBefore": "2020-08-26T11:15:39Z"} + {"rel_before":"12"}, + {"abs_before": "2020-08-26T11:15:39Z"} ] }, { diff --git a/xdr/json.go b/xdr/json.go index 50b4fb80db..47acc3ed77 100644 --- a/xdr/json.go +++ b/xdr/json.go @@ -11,8 +11,8 @@ type claimPredicateJSON struct { Or *[]claimPredicateJSON `json:"or,omitempty"` Not *claimPredicateJSON `json:"not,omitempty"` Unconditional bool `json:"unconditional,omitempty"` - AbsBefore *time.Time `json:"absBefore,omitempty"` - RelBefore *int64 `json:"relBefore,string,omitempty"` + AbsBefore *time.Time `json:"abs_before,omitempty"` + RelBefore *int64 `json:"rel_before,string,omitempty"` } func convertPredicatesToXDR(input []claimPredicateJSON) ([]ClaimPredicate, error) { diff --git a/xdr/json_test.go b/xdr/json_test.go index 83bdd9bc1a..02c3fd8add 100644 --- a/xdr/json_test.go +++ b/xdr/json_test.go @@ -41,7 +41,7 @@ func TestClaimPredicateJSON(t *testing.T) { assert.NoError(t, err) assert.JSONEq( t, - `{"and":[{"or":[{"relBefore":"12"},{"absBefore":"2020-08-26T11:15:39Z"}]},{"not":{"unconditional":true}}]}`, + `{"and":[{"or":[{"rel_before":"12"},{"abs_before":"2020-08-26T11:15:39Z"}]},{"not":{"unconditional":true}}]}`, string(serialized), ) From b51b25c66d4c9e6d806a64916dfec107a5139bfa Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 2 Oct 2020 10:30:14 -0500 Subject: [PATCH 2/6] Wrap invalid cursor in claimable balances query in problem.P (#3088) We do cursor validation, however, if there is an error we return a plain error instead of a `problem.P` -- this causes horizon to handle the error as a 500 instead of a 400. This fix moves cursor parsing to its own method and then use it in the query handler to return a `problem.P` error. Fixes https://github.com/stellar/go/issues/3085. --- .../internal/actions/claimable_balance.go | 8 +++ .../actions/claimable_balance_test.go | 54 ++++++++++++++++--- .../db2/history/claimable_balances.go | 27 +++++++--- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/services/horizon/internal/actions/claimable_balance.go b/services/horizon/internal/actions/claimable_balance.go index a9e2e45ffc..cbec9b8ba4 100644 --- a/services/horizon/internal/actions/claimable_balance.go +++ b/services/horizon/internal/actions/claimable_balance.go @@ -155,6 +155,14 @@ func (handler GetClaimableBalancesHandler) GetResourcePage( Claimant: qp.claimant(), } + _, _, err = query.Cursor() + if err != nil { + return nil, problem.MakeInvalidFieldProblem( + "cursor", + errors.New("First part should be higher than 0 and second part should be valid claimable balance ID"), + ) + } + historyQ, err := horizonContext.HistoryQFromRequest(r) if err != nil { return nil, err diff --git a/services/horizon/internal/actions/claimable_balance_test.go b/services/horizon/internal/actions/claimable_balance_test.go index d76e34a13a..082e23f71b 100644 --- a/services/horizon/internal/actions/claimable_balance_test.go +++ b/services/horizon/internal/actions/claimable_balance_test.go @@ -524,12 +524,54 @@ func TestGetClaimableBalances(t *testing.T) { tt.Assert.NoError(err) tt.Assert.Len(response, 2) - // for _, resource := range response { - // tt.Assert.Equal( - // "GC3C4AKRBQLHOJ45U4XG35ESVWRDECWO5XLDGYADO6DPR3L7KIDVUMML", - // resource.(protocol.ClaimableBalance).Sponsor, - // ) - // } +} + +func TestCursorAndOrderValidation(t *testing.T) { + tt := test.Start(t) + defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) + q := &history.Q{tt.HorizonSession()} + + handler := GetClaimableBalancesHandler{} + _, err := handler.GetResourcePage(httptest.NewRecorder(), makeRequest( + t, + map[string]string{ + "cursor": "-1-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18", + }, + map[string]string{}, + q.Session, + )) + p := err.(*problem.P) + tt.Assert.Equal("bad_request", p.Type) + tt.Assert.Equal("cursor", p.Extras["invalid_field"]) + tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"]) + + _, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest( + t, + map[string]string{ + "cursor": "1003529-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18", + }, + map[string]string{}, + q.Session, + )) + p = err.(*problem.P) + tt.Assert.Equal("bad_request", p.Type) + tt.Assert.Equal("cursor", p.Extras["invalid_field"]) + tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"]) + + _, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest( + t, + map[string]string{ + "order": "arriba", + "cursor": "1003529-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18", + }, + map[string]string{}, + q.Session, + )) + p = err.(*problem.P) + tt.Assert.Equal("bad_request", p.Type) + tt.Assert.Equal("order", p.Extras["invalid_field"]) + tt.Assert.Equal("order: invalid value", p.Extras["reason"]) } func TestClaimableBalancesQueryURLTemplate(t *testing.T) { diff --git a/services/horizon/internal/db2/history/claimable_balances.go b/services/horizon/internal/db2/history/claimable_balances.go index d72b928645..18665481af 100644 --- a/services/horizon/internal/db2/history/claimable_balances.go +++ b/services/horizon/internal/db2/history/claimable_balances.go @@ -22,10 +22,8 @@ type ClaimableBalancesQuery struct { Claimant *xdr.AccountId } -// ApplyCursor applies cursor to the given sql. For performance reason the limit -// is not apply here. This allows us to hint the planner later to use the right -// indexes. -func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBuilder, error) { +// Cursor validates and returns the query page cursor +func (cbq ClaimableBalancesQuery) Cursor() (int64, *xdr.ClaimableBalanceId, error) { p := cbq.PageQuery var l int64 var r *xdr.ClaimableBalanceId @@ -34,24 +32,37 @@ func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBu if p.Cursor != "" { parts := strings.SplitN(p.Cursor, "-", 2) if len(parts) != 2 { - return sql, errors.New("Invalid cursor") + return l, r, errors.New("Invalid cursor") } l, err = strconv.ParseInt(parts[0], 10, 64) if err != nil { - return sql, errors.Wrap(err, "Invalid cursor - first value should be higher than 0") + return l, r, errors.Wrap(err, "Invalid cursor - first value should be higher than 0") } var balanceID xdr.ClaimableBalanceId if err = xdr.SafeUnmarshalHex(parts[1], &balanceID); err != nil { - return sql, errors.Wrap(err, "Invalid cursor - second value should be a valid claimable balance id") + return l, r, errors.Wrap(err, "Invalid cursor - second value should be a valid claimable balance id") } r = &balanceID if l < 0 { - return sql, errors.Wrap(err, "Invalid cursor - first value should be higher than 0") + return l, r, errors.Wrap(err, "Invalid cursor - first value should be higher than 0") } } + return l, r, nil +} + +// ApplyCursor applies cursor to the given sql. For performance reason the limit +// is not apply here. This allows us to hint the planner later to use the right +// indexes. +func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBuilder, error) { + p := cbq.PageQuery + l, r, err := cbq.Cursor() + if err != nil { + return sql, err + } + switch p.Order { case db2.OrderAscending: if l > 0 && r != nil { From f84e85b6bc919be02434c29e205022e5a9860629 Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 2 Oct 2020 14:05:36 -0500 Subject: [PATCH 3/6] Fix typo on cursor error and improve message. (#3091) --- services/horizon/internal/actions/claimable_balance.go | 2 +- services/horizon/internal/actions/claimable_balance_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/horizon/internal/actions/claimable_balance.go b/services/horizon/internal/actions/claimable_balance.go index cbec9b8ba4..92de5f3edc 100644 --- a/services/horizon/internal/actions/claimable_balance.go +++ b/services/horizon/internal/actions/claimable_balance.go @@ -159,7 +159,7 @@ func (handler GetClaimableBalancesHandler) GetResourcePage( if err != nil { return nil, problem.MakeInvalidFieldProblem( "cursor", - errors.New("First part should be higher than 0 and second part should be valid claimable balance ID"), + errors.New("The first part should be a number higher than 0 and the second part should be a valid claimable balance ID"), ) } diff --git a/services/horizon/internal/actions/claimable_balance_test.go b/services/horizon/internal/actions/claimable_balance_test.go index 082e23f71b..d50f27ac53 100644 --- a/services/horizon/internal/actions/claimable_balance_test.go +++ b/services/horizon/internal/actions/claimable_balance_test.go @@ -544,7 +544,7 @@ func TestCursorAndOrderValidation(t *testing.T) { p := err.(*problem.P) tt.Assert.Equal("bad_request", p.Type) tt.Assert.Equal("cursor", p.Extras["invalid_field"]) - tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"]) + tt.Assert.Equal("The first part should be a number higher than 0 and the second part should be a valid claimable balance ID", p.Extras["reason"]) _, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest( t, @@ -557,7 +557,7 @@ func TestCursorAndOrderValidation(t *testing.T) { p = err.(*problem.P) tt.Assert.Equal("bad_request", p.Type) tt.Assert.Equal("cursor", p.Extras["invalid_field"]) - tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"]) + tt.Assert.Equal("The first part should be a number higher than 0 and the second part should be a valid claimable balance ID", p.Extras["reason"]) _, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest( t, From 02dafc4a034bc98471f7028b92cb08bc55fc5035 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Tue, 6 Oct 2020 17:53:29 +0200 Subject: [PATCH 4/6] services/horizon: Add claimable_balances table to state tables to truncate (#3100) --- services/horizon/internal/db2/history/ingestion.go | 1 + services/horizon/internal/expingest/main.go | 4 +++- services/horizon/internal/expingest/verify.go | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/services/horizon/internal/db2/history/ingestion.go b/services/horizon/internal/db2/history/ingestion.go index 3eda506d90..a2c6094a7a 100644 --- a/services/horizon/internal/db2/history/ingestion.go +++ b/services/horizon/internal/db2/history/ingestion.go @@ -10,6 +10,7 @@ func (q *Q) TruncateExpingestStateTables() error { "accounts", "accounts_data", "accounts_signers", + "claimable_balances", "exp_asset_stats", "offers", "trust_lines", diff --git a/services/horizon/internal/expingest/main.go b/services/horizon/internal/expingest/main.go index ee5d54be6d..80495f484a 100644 --- a/services/horizon/internal/expingest/main.go +++ b/services/horizon/internal/expingest/main.go @@ -46,7 +46,9 @@ const ( // - 10: Fixes a bug in meta processing (fees are now processed before // everything else). // - 11: Protocol 14: CAP-23 and CAP-33. - CurrentVersion = 11 + // - 12: Trigger state rebuild due to `absTime` -> `abs_time` rename + // in ClaimableBalances predicates. + CurrentVersion = 12 // MaxDBConnections is the size of the postgres connection pool dedicated to Horizon ingestion: // * Ledger ingestion, diff --git a/services/horizon/internal/expingest/verify.go b/services/horizon/internal/expingest/verify.go index 2ee8173a41..dd1c3804e4 100644 --- a/services/horizon/internal/expingest/verify.go +++ b/services/horizon/internal/expingest/verify.go @@ -27,7 +27,7 @@ const assetStatsBatchSize = 500 // check them. // There is a test that checks it, to fix it: update the actual `verifyState` // method instead of just updating this value! -const stateVerifierExpectedIngestionVersion = 11 +const stateVerifierExpectedIngestionVersion = 12 // verifyState is called as a go routine from pipeline post hook every 64 // ledgers. It checks if the state is correct. If another go routine is already From acd2b811d8ad60f96c1262eeba4e3b585585e35b Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Thu, 8 Oct 2020 12:28:23 +0200 Subject: [PATCH 5/6] services/horizon: Add Go and process related metrics (#3103) --- services/horizon/internal/app.go | 6 ++++++ services/horizon/internal/init.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/services/horizon/internal/app.go b/services/horizon/internal/app.go index fadb8fa222..f94699e1c5 100644 --- a/services/horizon/internal/app.go +++ b/services/horizon/internal/app.go @@ -440,6 +440,12 @@ func (a *App) init() error { a.prometheusRegistry.MustRegister(meter) } + // go metrics + initGoMetrics(a) + + // process metrics + initProcessMetrics(a) + // db-metrics initDbMetrics(a) diff --git a/services/horizon/internal/init.go b/services/horizon/internal/init.go index 952cdb9d80..fbc9e12001 100644 --- a/services/horizon/internal/init.go +++ b/services/horizon/internal/init.go @@ -215,6 +215,21 @@ func initDbMetrics(app *App) { app.prometheusRegistry.MustRegister(app.orderBookStream.LatestLedgerGauge) } +// initGoMetrics registers the Go collector provided by prometheus package which +// includes Go-related metrics. +func initGoMetrics(app *App) { + app.prometheusRegistry.MustRegister(prometheus.NewGoCollector()) +} + +// initProcessMetrics registers the process collector provided by prometheus +// package. This is only available on operating systems with a Linux-style proc +// filesystem and on Microsoft Windows. +func initProcessMetrics(app *App) { + app.prometheusRegistry.MustRegister( + prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{}), + ) +} + // initIngestMetrics registers the metrics for the ingestion into the provided // app's metrics registry. func initIngestMetrics(app *App) { From 38cc1f6394e32f1d9a34cf3916a7f429d0353abf Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Tue, 13 Oct 2020 20:53:39 +0200 Subject: [PATCH 6/6] services/horizon: Horizon 1.10.0 CHANGELOG (#3124) --- services/horizon/CHANGELOG.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index 6f4984593c..5abfd4b78b 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -3,9 +3,16 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/).x -## Unreleased +## v1.10.0 -* Dropped support for Go 1.13. +**After upgrading Horizon will rebuild its state. During this process (which can take several minutes) it will not ingest new ledgers.** + +* Fixed a bug that caused a fresh instance of Horizon to be unable to sync with testnet (Protocol 14) correctly. ([#3100](https://github.com/stellar/go/pull/3100)) +* Add Golang- and process-related metrics. ([#3103](https://github.com/stellar/go/pull/3103)) +* New `network_passphrase` field in History Archives (added in Stellar-Core 14.1.0) is now checked. Horizon will return error if incorrect archive is used. ([#3082](https://github.com/stellar/go/pull/3082)) +* Fixed a bug that caused some errors to be logged with `info` level instead of `error` level. ([#3094](https://github.com/stellar/go/pull/3094)) +* Fixed a bug in `/claimable_balances` that returned 500 error instead of 400 for some requests. ([#3088](https://github.com/stellar/go/pull/3088)) +* Print a friendly message when Horizon does not support the current Stellar protocol version. ([#3093](https://github.com/stellar/go/pull/3093)) ## v1.9.1