stellar-deprecated / kelp

Kelp is a free and open-source trading bot for the Stellar DEX and 100+ centralized exchanges
https://kelpbot.io
Other
1.1k stars 261 forks source link

Fill Tracker cannot unmarshal string into Go struct field Price._embedded.records.price.n of type int32 #741

Closed nikhilsaraf closed 3 years ago

nikhilsaraf commented 3 years ago

Describe the bug

Getting the following error in the latest build of Kelp when trying to fetch trades:

error when running an iteration of fill tracker: error when fetching trades: error while fetching trades in SDEX (cursor=???????????????-?): json: cannot unmarshal string into Go struct field Price._embedded.records.price.n of type int32

Please provide us with the relevant information in this quick form (1 minute) so we can track who is being faced with an outage on Kelp because of the Protocol 18 upgrade, and can help you get back online as quickly as possible https://forms.gle/G1Ud6HZjubaP6qS1A


Expected behavior

I think the correct behavior in this situation is for the bot to parse trade values (and price) correctly

Frequency

The frequency is: since at least 2021/10/06 17:48:15 UTC

Steps To Reproduce

  1. Run Kelp on SDEX with fill tracking enabled.
  2. Execute a trade against your bot.
  3. When the fill tracker runs, it will try and parse the trade and will fail with the above error

Possible Solution

This bug can be fixed by patching the price parsing logic by either upgrading the library or running a parallel library import with the fix (since we are using an old version of the SDK)

Your Environment

All versions of Kelp as off October 6th 2021

Context

I am unable to run any trading template that uses the fill tracker such as the balanced or mirror templates because of this bug.

Attachments

None

References

See this PR 3915 in the stellar/go repo for the change made to horizon need to switch over from using Price to using TradePrice

See updated Go SDK: https://github.com/stellar/go/releases/tag/horizonclient-v8.0.0-beta.0

Version of horizon released: https://github.com/stellar/go/releases/tag/horizon-v2.9.0rc1

nikhilsaraf commented 3 years ago

Please provide us with the relevant information in this quick form (1 minute) so we can track who is being faced with an outage on Kelp because of the Protocol 18 upgrade, and can help you get back online as quickly as possible

https://forms.gle/G1Ud6HZjubaP6qS1A

cyborr commented 3 years ago

After network upgrade KELP bot is crashing.

syllik commented 3 years ago

Hello there. Would be nice to get info, when this bug can be fixed. Best regards.

ddombrowsky commented 3 years ago

@nikhilsaraf please let me know if I can help. I have a devenv for kelp already.

minherc commented 3 years ago

the DEX market emptied sharply ) really looking forward to the patch!

itsseanbennett commented 3 years ago

If you're looking to just get back into the market and don't mind a temporary hack, I took stellar/go @ tag horizonclient-v3.0.0 and copied in the stellar/go/build package directly into Kelp. Some functionality (like auto trust lines) I just commented out entirely to save time but most changes to make the code fit in seemed OK - I'm testing it now - would anyone be interested in this?

ddombrowsky commented 3 years ago

If you're looking to just get back into the market and don't mind a temporary hack, I took stellar/go @ tag horizonclient-v3.0.0 and copied in the stellar/go/build package directly into Kelp. Some functionality (like auto trust lines) I just commented out entirely to save time but most changes to make the code fit in seemed OK - I'm testing it now - would anyone be interested in this?

I tried patching in the commit referenced earler into horizonclient-v3.0.0, but I like your solution better. That way upstream stays pristine.

For the record, git patch to github.com/stellar/go @ horizonclient-v3.0.0 is https://gist.github.com/ddombrowsky/24de38de1ed49bb5d863faa01fdfcaf3 . You then have to change go.mod to point to the local repo. It does compile and run, but it ain't pretty.

itsseanbennett commented 3 years ago

If you're looking to just get back into the market and don't mind a temporary hack, I took stellar/go @ tag horizonclient-v3.0.0 and copied in the stellar/go/build package directly into Kelp. Some functionality (like auto trust lines) I just commented out entirely to save time but most changes to make the code fit in seemed OK - I'm testing it now - would anyone be interested in this?

I tried patching in the commit referenced earler into horizonclient-v3.0.0, but I like your solution better. That way upstream stays pristine.

For the record, git patch to github.com/stellar/go @ horizonclient-v3.0.0 is https://gist.github.com/ddombrowsky/24de38de1ed49bb5d863faa01fdfcaf3 . You then have to change go.mod to point to the local repo. It does compile and run, but it ain't pretty.

I like what you've done here. It does involve editing the dependency but it seems cleaner - my change requires changing kelp files. I think people should try your route.

minherc commented 3 years ago

leighmcculloch, do not wait for a quick patch? strange somehow it turned out, about protocol 18 was out of place for a long time, but nothing is ready for it ) for us, for example, kelp worked for production ) can it be replaced with something?

minherc commented 3 years ago

If you're looking to just get back into the market and don't mind a temporary hack, I took stellar/go @ tag horizonclient-v3.0.0 and copied in the stellar/go/build package directly into Kelp. Some functionality (like auto trust lines) I just commented out entirely to save time but most changes to make the code fit in seemed OK - I'm testing it now - would anyone be interested in this?

I tried patching in the commit referenced earler into horizonclient-v3.0.0, but I like your solution better. That way upstream stays pristine.

For the record, git patch to github.com/stellar/go @ horizonclient-v3.0.0 is https://gist.github.com/ddombrowsky/24de38de1ed49bb5d863faa01fdfcaf3 . You then have to change go.mod to point to the local repo. It does compile and run, but it ain't pretty.

Please describe in detail how to use it. How do we get a working KELP? ps in GO we understand very little.

minherc commented 3 years ago

If you're looking to just get back into the market and don't mind a temporary hack, I took stellar/go @ tag horizonclient-v3.0.0 and copied in the stellar/go/build package directly into Kelp. Some functionality (like auto trust lines) I just commented out entirely to save time but most changes to make the code fit in seemed OK - I'm testing it now - would anyone be interested in this?

Please describe in steps, in detail how to use it. How do we get a working KELP? ps: in GO we understand very little.

ddombrowsky commented 3 years ago

@itsseanbennett

I like what you've done here. It does involve editing the dependency but it seems cleaner - my change requires changing kelp files. I think people should try your route.

It comes down to which repo you want to fork: stellar-go sdk or kelp. It seems it would be more correct (or perhaps, less wrong) to use the upstream SDK and patch kelp. However, it looks like the build sdk was deprecated in favor of txnbuild (https://github.com/stellar/go/blob/horizonclient-v3.0.0/build/README.md), so perhaps it is best to use a patched SDK until kelp can be properly updated to use txnbuild. What does @nikhilsaraf think?

minherc commented 3 years ago

@itsseanbennett

I like what you've done here. It does involve editing the dependency but it seems cleaner - my change requires changing kelp files. I think people should try your route.

It comes down to which repo you want to fork: stellar-go sdk or kelp. It seems it would be more correct (or perhaps, less wrong) to use the upstream SDK and patch kelp. However, it looks like the build sdk was deprecated in favor of txnbuild (https://github.com/stellar/go/blob/horizonclient-v3.0.0/build/README.md), so perhaps it is best to use a patched SDK until kelp can be properly updated to use txnbuild. What does @nikhilsaraf think?

@ddombrowsky Could it be easier for now to make a patch for KELP? Just fix the dependencies that are not going away now. So that KELP can work. Then do everything at the kernel and core level (sdk) Of course, I don't know the exact data architecture in stellar. Maybe just make a patch on the fly that will convert into the data that KELP wants to receive? (You can also hardcode, just that would work)

ps: or this question was not for me) pps: if possible, please write step by step how to build with the necessary changes KELP (better, of course, a bash script)

nikhilsaraf commented 3 years ago

I am preparing the repo for release, so you can expect a pre-compiled binary today. Apologies for the outage and thank you all for your patience! 🙏

nikhilsaraf commented 3 years ago

new release available here: https://github.com/stellar/kelp/releases/tag/v1.12.0

minherc commented 3 years ago

new release available here: https://github.com/stellar/kelp/releases/tag/v1.12.0

Thank you very much! We have already updated everything, everything is working.

We wish you all the best and success in your creative work.