Skip to content

Rest in taxi#292

Closed
bordalix wants to merge 10 commits intovulpemventures:masterfrom
bordalix:rest_in_taxi
Closed

Rest in taxi#292
bordalix wants to merge 10 commits intovulpemventures:masterfrom
bordalix:rest_in_taxi

Conversation

@bordalix
Copy link
Copy Markdown
Contributor

Fixes #290

Access taxi services via REST (removes gRPC).
Remove taxi-protobuf dependency.
Throw response (and not response.data) on broadcastTx.

@tiero please review

@tiero
Copy link
Copy Markdown
Member

tiero commented Jan 20, 2022

testing on Chrome I am getting gRPC requires HTTP/2. This most likely beacuse you are not passing content type application/json. We multiplex on the same port many protocols and switch based on content type.

return res.toObject();
assetHash: string
): Promise<TopupWithAssetReply> => {
const { data } = await axios.post(`${taxiUrl}/asset/topup`, assetHash);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We miss third parameter, with an object and a key headers where we specify application/json

strange works for you

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was working because I had { assetHash } has second parameter, and when we have an object has second parameter axios adds the content-type application/json by default. My last commit introduced the error, was not properly tested. This new commit 9eaef50 fixes it. @tiero can you try it?

@tiero
Copy link
Copy Markdown
Member

tiero commented Jan 21, 2022

I tried to send 1USDt but seems it multiples wrongly to be used for coin selection

Error: Not enought coins to select 10000000049500000 f3d1ec678811398cd2ae277cbe3849c6f6dbd72c74bc542f7c4b11ff0e820958 (has: 316666666)

Screenshot 2022-01-21 at 11 08 50

Screenshot 2022-01-21 at 11 08 57

@bordalix
Copy link
Copy Markdown
Contributor Author

I tried to send 1USDt but seems it multiples wrongly to be used for coin selection

Error: Not enought coins to select 10000000049500000 f3d1ec678811398cd2ae277cbe3849c6f6dbd72c74bc542f7c4b11ff0e820958 (has: 316666666)

Yes, yesterday I notice this also, but it seems to happen on master also (can you confirm?) so it seems not related with this PR. Also, I have an idea this is addressed on @louisinger PR.

@tiero
Copy link
Copy Markdown
Member

tiero commented Jan 21, 2022

werid since I can't test the taxi topup then

@tiero
Copy link
Copy Markdown
Member

tiero commented Jan 24, 2022

but it seems to happen on master also (can you confirm?)

master seems to have outdate taxi-protobuf, therefore cant ListAssets correctly and test taxi coin selection

@bordalix
Copy link
Copy Markdown
Contributor Author

I'm working with @louisinger to have PR #244 stable (and with Taxi REST) in order for us to be able to test it and finally merge it.

@tiero tiero closed this Jan 31, 2022
@bordalix bordalix deleted the rest_in_taxi branch February 3, 2022 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Taxi connection is not working

2 participants