Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Increase user-friendliness of short of assets error (closes #104) #492

Closed
wants to merge 3 commits into from

Conversation

debnil
Copy link
Contributor

@debnil debnil commented Aug 28, 2020

This PR increases the user-friendliness of the error when short of assets. This closes #104.

@debnil debnil self-assigned this Aug 28, 2020
Copy link
Contributor

@nikhilsaraf nikhilsaraf left a comment

Choose a reason for hiding this comment

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

let me know if you have any questions.

@@ -351,7 +351,7 @@ func (s *sellSideStrategy) computeRemainderAmount(incrementalSellAmount float64,
}

if availableSellingCapacity.Selling >= incrementalSellAmount && availableBuyingCapacity.Buying >= incrementalBuyAmount {
return 0, 0, fmt.Errorf("error: (programmer?) unable to create offer but available capacities were more than the attempted offer amounts, sellingCapacity=%.8f, incrementalSellAmount=%.8f, buyingCapacity=%.8f, incrementalBuyAmount=%.8f",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this old error message is correct. See the if condition -- it should always hold true. If it is false then it means something has gone terribly wrong and it's likely a programmer error as indicated by the error message.

IIRC this error has always occurred whenever there were less assets in the user's account so this error message was only a red herring.

There is another error message that indicates that the user has less assets. Search for the following in ieif.go:
we will oversell

There are two occurrences of that message, we should leave those two error messages as-is and add this additional line to hint to the user the corrective action they need to take:

utils.PrintErrorHintf("Your account may have run out of the asset '%s'. Add more tokens of '%s' so the bot can run correctly")

You will need to reproduce this issue to confirm that it is working correctly. Once fixed, could you please upload the section of the log file (between the two separator lines --------) when the user has a low balance for a bot trading (a) SOME_ASSET_A vs XLM where XLM is low; and (b) market trading with SOME_ASSET_A vs. SOME_ASSET_B where XLM is low and it is unable to pay fees?

@debnil
Copy link
Contributor Author

debnil commented Aug 29, 2020

@nikhilsaraf: Got it, thanks!! Would you be able to write e steps to reproduce the ieif.go error on the original issue, just to make sure we're on the same page?

@nikhilsaraf
Copy link
Contributor

Added some details on the issue. Let me know if that helps or we can discuss on Monday too.

@debnil
Copy link
Contributor Author

debnil commented Aug 31, 2020

Those are super helpful. I'm just going to close this PR - the changes are super different, and this way, I can contribute from a fork rather than a branch pushed to master.

@debnil debnil closed this Aug 31, 2020
@debnil debnil deleted the debnil/fix-sellside-error branch August 31, 2020 05:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[3] If short of assets, the error message should be a lot more user friendly
3 participants