API server #323

jackzampolin · 2020-11-06T02:29:59Z

cc @anilcse @akhilkumarpilli

See the list of routes as well as comments

ref: #52

akhilkumarpilli · 2020-11-10T08:47:19Z

@jackzampolin, Can you share me checklist of api endpoints here so it would be easy for us to implement them.

jackzampolin · 2020-11-11T19:12:15Z

@akhilkumarpilli Done! I've got them all stubbed out

anilcse · 2020-12-18T19:22:07Z

@akhilkumarpilli can you fix the conflicts?

lgtm-com · 2020-12-19T11:13:57Z

This pull request introduces 7 alerts when merging 124eacd into 031ecc2 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

odeke-em

Thank you @akhilkumarpilli @jackzampolin for this change! Just a drive-by review for security and nits, I've added some comments, please take a look.

helpers/light.go

helpers/query.go

relayer/query.go

relayer/strategies.go

helpers/api.go

lgtm-com · 2020-12-22T08:14:24Z

This pull request introduces 7 alerts when merging 9aed68d into f8acf82 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

lgtm-com · 2020-12-22T11:49:08Z

This pull request introduces 7 alerts when merging 2e1b5e9 into f8acf82 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

lgtm-com · 2021-01-15T07:09:50Z

This pull request introduces 7 alerts when merging dea3362 into 978a993 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

colin-axner · 2021-01-18T15:13:30Z

I will review this sometime this week, just haven't gotten a chance yet since there are a lot of changes

colin-axner

Overall LGTM. I did my best to review this, but it is a lot of diffs, and most of it looks like a refactor of old function structuring. Just left a couple questions on the rest response writing

Would appreciate another reviewer to take a look at this, but it does only touch client code so it's ok if bugs are not caught during review.

@akhilkumarpilli have you tested this manually for a couple of the rest routes? Could you add a quick document on how to interact with this?

colin-axner · 2021-01-19T14:22:27Z

cmd/api.go

+			// Data for this should be stored in the ServicesManager struct
+			r.HandleFunc("/listen/{path}/{strategy}/{name}", PostRelayerListenHandler(sm)).Methods("POST")
+
+			// TODO: do we want to add the transaction commands here to?


client/connection/channel commands change the config during execution so we would want to make sure we are aware of that when addressing this TODO

@colin-axner , do we need to remove this TODO since we thought of not implementing APIS for tx commands?

lets remove

colin-axner · 2021-01-19T14:22:50Z

cmd/api.go

+			// tx transfer
+			// POST /paths/{name}/transfers
+
+			// TODO: listen validation in config


We should open issues for every TODO

helpers/query.go

helpers/api.go

colin-axner · 2021-01-27T11:17:07Z

also waiting on review from @fedekunze

lgtm-com · 2021-02-03T06:16:25Z

This pull request introduces 7 alerts when merging a237c8b into 2f1a812 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

lgtm-com · 2021-02-03T07:52:32Z

This pull request introduces 7 alerts when merging 16bf3df into 2f1a812 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

akhilkumarpilli · 2021-02-03T09:21:37Z

@akhilkumarpilli have you tested this manually for a couple of the rest routes? Could you add a quick document on how to interact with this?

@colin-axner , We will setup swagger docs for rest APIs in a separate PR.

colin-axner · 2021-02-03T11:39:35Z

Adding swagger docs later sounds fine, but I think someone should manually test to at least ensure one route works. Is there a reason why this cannot be done? Is it because we'd need some third party code to submit the rest requests?

akhilkumarpilli · 2021-02-03T11:50:51Z

Not a problem, I will add a short document regarding how to start rest server and interact with routes. We thought swagger docs might be helpful since we can test routes directly from it. We will add swagger docs later in other PR and push a quick document now.

colin-axner · 2021-02-03T11:53:31Z

sounds perfect, thanks @akhilkumarpilli

lgtm-com · 2021-02-04T05:58:05Z

This pull request introduces 7 alerts when merging bc84060 into cb3edfc - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

fedekunze

utACK, although I think this API could be further refactored / cleaned up. Can you also fix the alerts from lgtm-com?

fedekunze · 2021-02-04T11:50:10Z

cmd/api.go

+
+			// VERSION
+			// version get
+			r.HandleFunc(fmt.Sprintf("/%s", version), VersionHandler).Methods("GET")


these are really hard to read, what's the need for fmt.Sprintf? I think the value of having the endpoints as constants is less than using a plain string

cmd/keys.go

jackzampolin · 2021-02-05T00:42:21Z

cmd/api.go

+}
+
+// PostRelayerListenHandler returns a handler for a listener that can listen on many IBC paths
+func PostRelayerListenHandler(sm *ServicesManager) func(w http.ResponseWriter, r *http.Request) {


Does this work? That would a step change in the relayer capabilities.

We need to do some rework for this api. We can implement all TODOs related to PostRelayerListenHandler in separate PR.

akhilkumarpilli · 2021-02-08T07:58:20Z

Can you also fix the alerts from lgtm-com?

I tried to fix lgtm-com alerts but I am unable to fix them.

lgtm-com · 2021-02-08T08:13:48Z

This pull request introduces 7 alerts when merging 6a08698 into 9f13a93 - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

colin-axner · 2021-02-08T11:56:05Z

I tried to fix lgtm-com alerts but I am unable to fix them.

The incorrect conversion can be fixed by changing the upstream offset value to int instead of uint64 or by adding a if statement before doing QueryTxs action.

The uncontrolled data path I think we can just suppress those errors (click on the hide button on lgtm to see how to suppress). The suppression won't take place until after this is merged, but we should still add it

cmd/api.go

lgtm-com · 2021-02-11T11:09:57Z

This pull request introduces 7 alerts when merging 8ee029d into 872dc2e - view on LGTM.com

new alerts:

4 for Uncontrolled data used in path expression
2 for Incorrect conversion between integer types
1 for Uncontrolled data used in network request

helpers/query.go

colin-axner

utACK, I think we can merge this in so it starts getting some testing, see my MaxInt32 comment first though since it seems like a small fix

WIP API server

0fba0e3

Stub out API

1e6c39d

akhilkumarpilli added 6 commits November 21, 2020 16:40

Add keys and light get api queries

c9ca48a

Add query account and balance apis

4b56a71

Done get method api queries

3adbdd1

Add query txs and few key api queries

920cdbe

Modify overWriteConfig and add few chain api queries

affd582

Modularize api and add few more api queries

6ce98c3

anilcse changed the title ~~WIP API server~~ API server Dec 18, 2020

akhilkumarpilli added 2 commits December 19, 2020 16:22

Update akash version

481f2c1

Merge branch 'master' into jack/api

124eacd

odeke-em reviewed Dec 19, 2020

View reviewed changes

Address few PR comments

9aed68d

Handle start errors in strategy

2e1b5e9

Merge branch 'master' into jack/api

dea3362

akhilkumarpilli requested review from colin-axner and anilcse January 15, 2021 06:57

colin-axner requested review from fedekunze and AdityaSripal January 15, 2021 11:31

colin-axner reviewed Jan 22, 2021

View reviewed changes

Merge branch 'master' into jack/api

a237c8b

akhilkumarpilli added 2 commits February 3, 2021 12:54

Fix lint issues

207559e

Fix new lint issues

16bf3df

Add rest server doc

bc84060

fedekunze approved these changes Feb 4, 2021

View reviewed changes

jackzampolin commented Feb 5, 2021

View reviewed changes

akhilkumarpilli added 2 commits February 8, 2021 11:46

Merge branch 'master' into jack/api

42fb102

Address few suggestions

fa82305

Remove unused constants

6a08698

akhilkumarpilli mentioned this pull request Feb 8, 2021

Rest server for all CLI functionality #52

Closed

3 tasks

colin-axner requested a review from fedekunze February 8, 2021 11:56

colin-axner reviewed Feb 8, 2021

View reviewed changes

cmd/api.go Show resolved Hide resolved

akhilkumarpilli added 2 commits February 11, 2021 15:42

Merge branch 'master' into jack/api

b93f2ed

Add incorrect conversion alert check

8ee029d

colin-axner reviewed Feb 11, 2021

View reviewed changes

helpers/query.go Show resolved Hide resolved

colin-axner approved these changes Feb 11, 2021

View reviewed changes

colin-axner merged commit c690806 into master Feb 11, 2021

colin-axner deleted the jack/api branch February 11, 2021 12:33

AlianBenabdallah mentioned this pull request Jul 27, 2022

Investigate & document learnings from the Go relayer informalsystems/hermes#2426

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API server #323

API server #323

jackzampolin commented Nov 6, 2020 •

edited by akhilkumarpilli

Loading

akhilkumarpilli commented Nov 10, 2020

jackzampolin commented Nov 11, 2020

anilcse commented Dec 18, 2020

lgtm-com bot commented Dec 19, 2020

odeke-em left a comment

lgtm-com bot commented Dec 22, 2020

lgtm-com bot commented Dec 22, 2020

lgtm-com bot commented Jan 15, 2021

colin-axner commented Jan 18, 2021

colin-axner left a comment

colin-axner Jan 19, 2021

akhilkumarpilli Feb 2, 2021

colin-axner Feb 3, 2021

colin-axner Jan 19, 2021

colin-axner commented Jan 27, 2021

lgtm-com bot commented Feb 3, 2021

lgtm-com bot commented Feb 3, 2021

akhilkumarpilli commented Feb 3, 2021

colin-axner commented Feb 3, 2021

akhilkumarpilli commented Feb 3, 2021

colin-axner commented Feb 3, 2021

lgtm-com bot commented Feb 4, 2021

fedekunze left a comment •

edited

Loading

fedekunze Feb 4, 2021

jackzampolin Feb 5, 2021

akhilkumarpilli Feb 8, 2021

akhilkumarpilli commented Feb 8, 2021

lgtm-com bot commented Feb 8, 2021

colin-axner commented Feb 8, 2021

lgtm-com bot commented Feb 11, 2021

colin-axner left a comment •

edited

Loading

API server #323

API server #323

Conversation

jackzampolin commented Nov 6, 2020 • edited by akhilkumarpilli Loading

akhilkumarpilli commented Nov 10, 2020

jackzampolin commented Nov 11, 2020

anilcse commented Dec 18, 2020

lgtm-com bot commented Dec 19, 2020

odeke-em left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Dec 22, 2020

lgtm-com bot commented Dec 22, 2020

lgtm-com bot commented Jan 15, 2021

colin-axner commented Jan 18, 2021

colin-axner left a comment

Choose a reason for hiding this comment

colin-axner Jan 19, 2021

Choose a reason for hiding this comment

akhilkumarpilli Feb 2, 2021

Choose a reason for hiding this comment

colin-axner Feb 3, 2021

Choose a reason for hiding this comment

colin-axner Jan 19, 2021

Choose a reason for hiding this comment

colin-axner commented Jan 27, 2021

lgtm-com bot commented Feb 3, 2021

lgtm-com bot commented Feb 3, 2021

akhilkumarpilli commented Feb 3, 2021

colin-axner commented Feb 3, 2021

akhilkumarpilli commented Feb 3, 2021

colin-axner commented Feb 3, 2021

lgtm-com bot commented Feb 4, 2021

fedekunze left a comment • edited Loading

Choose a reason for hiding this comment

fedekunze Feb 4, 2021

Choose a reason for hiding this comment

jackzampolin Feb 5, 2021

Choose a reason for hiding this comment

akhilkumarpilli Feb 8, 2021

Choose a reason for hiding this comment

akhilkumarpilli commented Feb 8, 2021

lgtm-com bot commented Feb 8, 2021

colin-axner commented Feb 8, 2021

lgtm-com bot commented Feb 11, 2021

colin-axner left a comment • edited Loading

Choose a reason for hiding this comment

jackzampolin commented Nov 6, 2020 •

edited by akhilkumarpilli

Loading

fedekunze left a comment •

edited

Loading

colin-axner left a comment •

edited

Loading