300 mBTC Bounty - Clean up `AccountInfo` #1065

Closed
timmolter opened this Issue Sep 21, 2015 · 12 comments

Comments

Projects
None yet
6 participants
@timmolter
Member

timmolter commented Sep 21, 2015

figure out best Wallets solution. See:

Propose a good solution, address TODOs, remove deprecated code, refactor all implementations and example code as needed.

@timmolter timmolter added the Bounty label Sep 21, 2015

@rafalkrupinski

This comment has been minimized.

Show comment
Hide comment
@rafalkrupinski

rafalkrupinski Oct 1, 2015

Contributor

There is also @jheusser's input on user wallets in #919.

Contributor

rafalkrupinski commented Oct 1, 2015

There is also @jheusser's input on user wallets in #919.

@mmazi

This comment has been minimized.

Show comment
Hide comment
@mmazi

mmazi Oct 31, 2015

Collaborator

Another AccountInfo issue is #1072 .

Collaborator

mmazi commented Oct 31, 2015

Another AccountInfo issue is #1072 .

@baffo32

This comment has been minimized.

Show comment
Hide comment
@baffo32

baffo32 Nov 27, 2015

Contributor

Solution ideas:

  • AccountInfo gains member String walletId .
  • Account and Trade services may optionally implement an interface providing setWalletId(String walletId). Account service may also implement getAllWalletIds() and transferTo(String walletId, CurrencyPair currencyPair, BigDecimal amount). Their other methods act on the last set wallet id. Not many exchanges use these multiple wallets.
  • OkCoin's issue is only because it has not migrated to the solution agreed upon in #919 . This is just updating the code. balance = available + frozen - borrowed + loaned and/or available = balance - loaned + borrowed - frozen should be documented in AccountInfo. The equation should be kept correct whether or not borrowed and loaned are actually recorded with separate members.
  • Bitfinex has an issue where each wallet has differing functionality (one can trade, one can loan, one can borrow). I think here that each wallet could be implemented as a separate Exchange, resulting in 3 Bitfinex exchanges sharing some code underneath. Update: Alternatively the three wallets could be unified and offered as one. Implementation will have to handle possible race conditions shuffling money between them.
Contributor

baffo32 commented Nov 27, 2015

Solution ideas:

  • AccountInfo gains member String walletId .
  • Account and Trade services may optionally implement an interface providing setWalletId(String walletId). Account service may also implement getAllWalletIds() and transferTo(String walletId, CurrencyPair currencyPair, BigDecimal amount). Their other methods act on the last set wallet id. Not many exchanges use these multiple wallets.
  • OkCoin's issue is only because it has not migrated to the solution agreed upon in #919 . This is just updating the code. balance = available + frozen - borrowed + loaned and/or available = balance - loaned + borrowed - frozen should be documented in AccountInfo. The equation should be kept correct whether or not borrowed and loaned are actually recorded with separate members.
  • Bitfinex has an issue where each wallet has differing functionality (one can trade, one can loan, one can borrow). I think here that each wallet could be implemented as a separate Exchange, resulting in 3 Bitfinex exchanges sharing some code underneath. Update: Alternatively the three wallets could be unified and offered as one. Implementation will have to handle possible race conditions shuffling money between them.
@timmolter

This comment has been minimized.

Show comment
Hide comment
@timmolter

timmolter Dec 3, 2015

Member

@baffo32 Please provide a BTC address to collect the bounty.

Member

timmolter commented Dec 3, 2015

@baffo32 Please provide a BTC address to collect the bounty.

@baffo32

This comment has been minimized.

Show comment
Hide comment
@baffo32

baffo32 Dec 3, 2015

Contributor

1NiQsdUJdXh3z11DLaX3iT3QqoSYVGk2Gq

Contributor

baffo32 commented Dec 3, 2015

1NiQsdUJdXh3z11DLaX3iT3QqoSYVGk2Gq

@timmolter

This comment has been minimized.

Show comment
Hide comment
@timmolter

timmolter Dec 15, 2015

Member

@baffo32 Is this still the address you want paid to?

Member

timmolter commented Dec 15, 2015

@baffo32 Is this still the address you want paid to?

@xloem

This comment has been minimized.

Show comment
Hide comment
@xloem

xloem Dec 15, 2015

@timmolter thanks that should still work fine

xloem commented Dec 15, 2015

@timmolter thanks that should still work fine

@timmolter

This comment has been minimized.

Show comment
Hide comment
Member

timmolter commented Dec 15, 2015

@xloem are you @baffo32 ?

@baffo32

This comment has been minimized.

Show comment
Hide comment
@baffo32

baffo32 Dec 15, 2015

Contributor

sorry! yes that was me, good address still

Contributor

baffo32 commented Dec 15, 2015

sorry! yes that was me, good address still

@timmolter

This comment has been minimized.

Show comment
Hide comment
@timmolter

timmolter Dec 15, 2015

Member

Paid. Thanks!!!

Member

timmolter commented Dec 15, 2015

Paid. Thanks!!!

@currentsea

This comment has been minimized.

Show comment
Hide comment
@currentsea

currentsea Apr 2, 2016

just for your information -- it still says "open" on this page -- http://knowm.org/open-source/ :)

just for your information -- it still says "open" on this page -- http://knowm.org/open-source/ :)

@timmolter

This comment has been minimized.

Show comment
Hide comment
@timmolter

timmolter Apr 2, 2016

Member

@currentsea Thank! Fixed.

Member

timmolter commented Apr 2, 2016

@currentsea Thank! Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment