Page MenuHomePhabricator

[ecash-agora] Do not allow creation of unacceptable agora partials
ClosedPublic

Authored by bytesofman on Sat, Jan 11, 23:56.

Details

Summary

An agora partial offer that requires a min accept amount higher than the total amount of tokens offered cannot be accepted.

Throw an error in this case.

Note: this could have some downstream impact for users trying to accept an agora offer that creates a new offer for change using the same min accept params as the parent offer. Right now, it will make the offer and it will be unacceptable. Now it will throw an error...I think (I have not dug into this part of the code yet).

Cashtab handles this by preventing users from accepting a partial offer that would create an unacceptable offer as change. I think the more elegant way to handle it is to create the change offer but set the min buy to the change amount, if the change amount is lower than the parent min buy...though this may be impractical for the price impacts it would have.

Either way, we never want unacceptable offers to be created. This is a step toward that goal.

Note

I have now changed the unit tests in this lib from the original tests for two conditions.

earlier diff - min buy must be at least dust satoshis
this diff - we cannot create an offer if min accept is greater than offered tokens

These also ended up changing what the original tests were probably exploring, i.e. edge cases of very low offers. However in both cases, we do not want these offers to ever exist.

Test Plan

npm test, any feedback on potential downstream impacts or strategy

Diff Detail

Repository
rABC Bitcoin ABC
Branch
agora-no-unacceptable-partials
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32079
Build 63650: Build Diffecash-agora-integration-tests · ecash-agora-tests · cashtab-tests
Build 63649: arc lint + arc unit

Event Timeline

Fabien added inline comments.
modules/ecash-agora/src/partial.ts
346 ↗(On Diff #52145)

Is that the error that is shown to the user ?

bytesofman added inline comments.
modules/ecash-agora/src/partial.ts
346 ↗(On Diff #52145)

Yes

In this case, "users" are devs who are calling the AgoraPartial.approximateParams method. offeredTokens and minAcceptedTokens are the names of the params expected by the method.

We could also print the values being called, but I figured since the users are already using the function and need to call it with these params in an object that has these keys -- probably not too much ambiguity.

This revision is now accepted and ready to land.Mon, Jan 13, 15:08
bytesofman marked an inline comment as done.

rebase and confirm no git issue (dropped an earlier commit)