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.

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

Add Graphql support to scribe's data #165

Merged
merged 59 commits into from
Sep 7, 2022
Merged

Add Graphql support to scribe's data #165

merged 59 commits into from
Sep 7, 2022

Conversation

CryptoMaxPlanck
Copy link
Contributor

@CryptoMaxPlanck CryptoMaxPlanck commented Sep 5, 2022

Description
The Scribe will go through the history of chains and backfill data into a database. However, for this data to be useful, an API around it must be build to easily retrieve data. This PR uses GraphQL to build an API around the scribe's data.

Todo:

  • Deprecate database retrieval functions in favor of one with filter
  • fix context issues
  • fix generate
  • fix closing of request and listenandserve
  • add resolver tests

@github-actions github-actions bot added the go Pull requests that update Go code label Sep 5, 2022
"github.com/integralist/go-findroot/find"
)

func main() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the generate from this needs to be fixed. is popping up files in the wrong dir.


fmt.Printf("started graphiql server on port: http://localhost:%d/graphiql\n", port)

// TODO: respect context cancellation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to figure out how to do this

Copy link
Contributor

Choose a reason for hiding this comment

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

thsi is a bit of a pain. Might make sense for you to add this to core as a server functionality an dimport from there.

var lc net.ListenConfig
host := fmt.Sprintf("0.0.0.0:%d", s.config.RPCPort)
s.listener, err = lc.Listen(ctx, "tcp", host)
if err != nil {
	return fmt.Errorf("could not listen on %s: %w", host, err)
}

go func() {
	err := s.server.Serve(s.listener)
	if err != nil {
		logger.Errorf(fmt.Sprintf("rpc server got error: %v", err))
	}
}()

select {
case <-ctx.Done():
     return
}

func initDB(database string, path string) (db.EventDB, error) {
switch {
case database == "sqlite":
sqliteStore, err := sqlite.NewSqliteStore(context.TODO(), path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

what context am i supposed to use here if start doesn't have one? do I just make a new one with a timeout and pass that in to make sure the function doesn't take more than x seconds (like 5 seconds)

Copy link
Contributor

Choose a reason for hiding this comment

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

should be the one from start

var portFlag = &cli.UintFlag{
Name: "port",
Usage: "--port 5121",
//nolint:staticcheck
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this okay to use even though it is depracated? the alternative is using net.listen with port of :0 and then getting that port. but then would need to add a function with panics in case of errors, so less clean.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably handle errors in an init here. Something like

func init() {
   ports := freeport.get(1)
   if len(port) > 0 {
      port = uint16(ports[0])
  }
}

var port uint16

// RetrieveEthTxsInRange retrieves eth transactions in a range.
RetrieveEthTxsInRange(ctx context.Context, ethTxFilter EthTxFilter, startBlock, endBlock uint64) ([]types.Transaction, error)

// TODO: Deprecate these methods in favor of the above methods.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will progressively go thru and deprecate these throughout scribe. just want to get ur review on it first.

Copy link
Contributor

Choose a reason for hiding this comment

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

sick

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #165 (d45447d) into master (8d51934) will decrease coverage by 0.10890%.
The diff coverage is 0.80645%.

@@                 Coverage Diff                 @@
##              master        #165         +/-   ##
===================================================
- Coverage   52.41622%   52.30732%   -0.10891%     
===================================================
  Files            185         185                 
  Lines           8236        8278         +42     
  Branches          83          83                 
===================================================
+ Hits            4317        4330         +13     
- Misses          3512        3541         +29     
  Partials         407         407                 
Impacted Files Coverage Δ
services/scribe/db/datastore/sql/base/log.go 0.00000% <0.00000%> (ø)
services/scribe/db/datastore/sql/base/receipt.go 0.00000% <0.00000%> (ø)
...rvices/scribe/db/datastore/sql/base/transaction.go 0.00000% <0.00000%> (ø)
services/scribe/backfill/contract.go 75.00000% <100.00000%> (+1.28204%) ⬆️
ethergo/chain/block.go 68.54839% <0.00000%> (+2.41935%) ⬆️
ethergo/signer/signer/kmssigner/signing.go 42.39130% <0.00000%> (+3.26087%) ⬆️
services/scribe/backfill/filter.go 74.69880% <0.00000%> (+6.02410%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

i.Eventually(func() bool {
request, err := http.NewRequestWithContext(i.GetTestContext(), http.MethodGet, fmt.Sprintf("%s%s", baseURL, server.GraphiqlEndpoint), nil)
Nil(i.T(), err)
_, err = i.gqlClient.Client.Client.Do(request)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to look into how to close this without messing up the tests. (or is ignoring safe??)

Copy link
Contributor

Choose a reason for hiding this comment

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

res, err = i.gqlClient.Client.Client.Do(request)
defer res.Close()

@CryptoMaxPlanck CryptoMaxPlanck marked this pull request as draft September 5, 2022 03:31
//
// It serves as dependency injection for your app, add any dependencies you require here.

// go:generate go run github.com/synapsecns/sanguine/services/scribe/server/contrib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

again, fix the generate stuff

}
}

func (r Resolver) buildLogFilter(contractAddress *string, blockNumber *int, txHash *string, txIndex *int, blockHash *string, index *int) db.LogFilter {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not a super clean way to do this, but not sure how else. maybe utilize optionals in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I like that. If we're going to do it, let's throw it in core and use it cross-lib from go-ethereum.

This will become much easier when ethereum fully adopts go 1.18 ethereum/go-ethereum#24549

var portFlag = &cli.UintFlag{
Name: "port",
Usage: "--port 5121",
//nolint:staticcheck
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably handle errors in an init here. Something like

func init() {
   ports := freeport.get(1)
   if len(port) > 0 {
      port = uint16(ports[0])
  }
}

var port uint16

Description: "starts a graphql server",
Flags: []cli.Flag{portFlag, dbFlag, pathFlag},
Action: func(c *cli.Context) error {
err := server.Start(uint16(c.Uint(portFlag.Name)), c.String(dbFlag.Name), c.String(pathFlag.Name))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should 100% have a context passed in

services/scribe/db/datastore/sql/base/log.go Show resolved Hide resolved
// RetrieveLogs retrieves all logs that match a tx hash and chain id.
func (s Store) RetrieveLogs(ctx context.Context, txHash common.Hash, chainID uint32) (logs []*types.Log, err error) {
func logFilterToQuery(logFilter db.LogFilter) Log {
return Log{
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work if certain fields are nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup. will be done in tests when RetrieveLogsByTxHash is deprecated cuz ill be using a filter w only two fields then.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, that going to be in this pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya, the logs_test file fully uses this now

func (s Store) RetrieveLogsInRange(ctx context.Context, logFilter db.LogFilter, startBlock, endBlock uint64) (logs []*types.Log, err error) {
dbLogs := []Log{}
queryFilter := logFilterToQuery(logFilter)
rangeQuery := BlockNumberFieldName + " BETWEEN ? AND ?"
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt.Sprintf("%s BETWEEN ? AND ?", BlockNumberFieldName)

func initDB(database string, path string) (db.EventDB, error) {
switch {
case database == "sqlite":
sqliteStore, err := sqlite.NewSqliteStore(context.TODO(), path)
Copy link
Contributor

Choose a reason for hiding this comment

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

should be the one from start

}

// Start starts the server and initializes the database.
func Start(port uint16, database string, path string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

should take a context, c *cli.Context is a context (var _ context.Context = &cli.Context) so that can be passed in here


fmt.Printf("started graphiql server on port: http://localhost:%d/graphiql\n", port)

// TODO: respect context cancellation
Copy link
Contributor

Choose a reason for hiding this comment

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

thsi is a bit of a pain. Might make sense for you to add this to core as a server functionality an dimport from there.

var lc net.ListenConfig
host := fmt.Sprintf("0.0.0.0:%d", s.config.RPCPort)
s.listener, err = lc.Listen(ctx, "tcp", host)
if err != nil {
	return fmt.Errorf("could not listen on %s: %w", host, err)
}

go func() {
	err := s.server.Serve(s.listener)
	if err != nil {
		logger.Errorf(fmt.Sprintf("rpc server got error: %v", err))
	}
}()

select {
case <-ctx.Done():
     return
}

}
}

func (r Resolver) buildLogFilter(contractAddress *string, blockNumber *int, txHash *string, txIndex *int, blockHash *string, index *int) db.LogFilter {
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I like that. If we're going to do it, let's throw it in core and use it cross-lib from go-ethereum.

This will become much easier when ethereum fully adopts go 1.18 ethereum/go-ethereum#24549

func (r Resolver) buildReceiptFilter(txHash *string, contractAddress *string, blockHash *string, blockNumber *int, transactionIndex *int) db.ReceiptFilter {
receiptFilter := db.ReceiptFilter{}
if txHash != nil {
receiptFilter.TxHash = *txHash
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure how I feel about the potential nil dereference here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there a scenario where the ref is not nil, but the de-ref is?

// RetrieveLogs retrieves all logs that match a tx hash and chain id.
func (s Store) RetrieveLogs(ctx context.Context, txHash common.Hash, chainID uint32) (logs []*types.Log, err error) {
func logFilterToQuery(logFilter db.LogFilter) Log {
return Log{
Copy link
Contributor

Choose a reason for hiding this comment

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

got it, that going to be in this pr?

}

// RetrieveLogsWithFilter retrieves all logs that match a filter.
func (s Store) RetrieveLogsWithFilter(ctx context.Context, logFilter db.LogFilter) (logs []*types.Log, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's handle in a future pr

"fmt"

"github.com/synapsecns/sanguine/services/scribe/graphql/server/graph/model"
resolvers "github.com/synapsecns/sanguine/services/scribe/graphql/server/graph/resolver"
Copy link
Contributor

Choose a reason for hiding this comment

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

see ci build issue

@trajan0x trajan0x merged commit 7d59e36 into master Sep 7, 2022
@trajan0x trajan0x deleted the feat/graphql branch September 7, 2022 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code M-ci Module: CI M-make
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants