Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor bounty.value_in_usdt into two functions #693

Closed
owocki opened this issue Mar 23, 2018 · 15 comments
Closed

refactor bounty.value_in_usdt into two functions #693

owocki opened this issue Mar 23, 2018 · 15 comments
Assignees

Comments

@owocki
Copy link
Contributor

owocki commented Mar 23, 2018

what

refactor bounty.value_in_usdt into two functions

  • value_in_usdt_now - returns the value in usdt now
  • value_in_usdt - return the value in usdt at the time of the created_on

other considerations

you might also have to edit the data retention policy in the cleanup_db_space script and possibly also import some historical ETH to USD data to the database.

please be mindful of performance -- dont hit an external API when these methods are called. instead, please leverage the ConversionRate object.

downstream implications

as for the ui, and what function should be used when:

activity_report should show the latter
bounty explorer stats module should show the latter
bounty detail pages should show the former

success criteria

  • value_in_usdt_now - returns the value in usdt now
  • value_in_usdt - return the value in usdt at the time of the created_on
  • value_in_usdt returns correct data for ETH bounties for the timeframe of 9/1/2016 to present
@gitcoinbot
Copy link
Member

This issue now has a funding of 0.12 ETH (56.58 USD @ $471.48/ETH) attached to it.

  • If you would like to work on this issue you can claim it here.
  • If you've completed this issue and want to claim the bounty you can do so here
  • Questions? Get help on the Gitcoin Slack
  • $2730.8 more Funded OSS Work Available at: https://gitcoin.co/explorer

@mbeacom
Copy link
Contributor

mbeacom commented Mar 27, 2018

@isaacadams Thanks for picking this up! Feel free to reach out to me if you have any questions or need assistance with this bounty.

@gitcoinbot
Copy link
Member

gitcoinbot commented Apr 1, 2018

Work has been started on the 0.12 ETH (56.06 USD @ $467.2/ETH) funding by:

  1. @cryptomental

Please work together and coordinate delivery of the issue scope. Gitcoin doesn't know enough about everyones skillsets / free time to say who should work on what, but we trust that the community is smart and well-intentioned enough to work together. As a general rule; if you start work first, youll be at the top of the above list ^^, and should have 'dibs' as long as you follow through.

On the above list? Please leave a comment to let the funder (@owocki) and the other parties involved what you're working, with respect to this issue and your plans to resolve it. If you don't leave a comment, the funder may expire your submission at their discretion.

@cryptomental
Copy link
Contributor

Hi @mbeacom , @owocki I noticed that the issue was put back to open. @isaacadams if you are still working on this, I will leave it to you. Otherwise I will be happy to jump in.

@owocki
Copy link
Contributor Author

owocki commented Apr 2, 2018

@isaacadams did you want to work on this still? please let us know today

@isaacadams
Copy link

@owocki @cryptomental @mbeacom

Hey fellas, sorry that I haven't been communicating! I am new to the community and wanted to try to finish up what I was doing but haven't had the time to do it. Go ahead and work on it as I am no longer able to do so for the time being.

@owocki
Copy link
Contributor Author

owocki commented Apr 2, 2018

thanks for letting us know @isaacadams -- hope to see you around again when things cool down for you

@cryptomental all you!

@cryptomental
Copy link
Contributor

Ok. Thanks a lot. I will pick it up tomorrow morning!

@cryptomental
Copy link
Contributor

I have a lot of fun diving in, I suspect that the first issue will take me a few days as I need to dive in and look around at the codebase.

I have so far set up the workspace, built docker images, refactored value_in_usdt into value_in_usdt_now, locally changed data retention policy to days_back = 365*3 , managed to fetch historic bounties using sync_geth mainnet 0 99999999999 and I am currently looking on how to best populate the ConversionRate with historic data for all past bounties as I added debug logs and I see only the most recent one plus then one I was able to trigger update manually using docker-compose exec web python3 app/manage.py get_prices
:

web_1      | DEBUG:economy.utils:<generator object QuerySet._iterator at 0x7f91bf5f7410>
web_1      | DEBUG:economy.utils:1.0 ETH => 407.23957561 USDT (2018-04-03 18:04:58.441848+00:00, poloniex) 21 seconds ago
web_1      | DEBUG:economy.utils:0.00245555702316532 ETH => 1.0 USDT (2018-04-03 18:04:58.441848+00:00, poloniex) 21 seconds ago
web_1      | DEBUG:economy.utils:1.0 ETH => 398.83722279 USDT (2018-04-03 14:20:06.904151+00:00, poloniex) 3 hours ago
web_1      |DEBUG:economy.utils:0.0025072885449474 ETH => 1.0 USDT (2018-04-03 14:20:06.904151+00:00, poloniex) 3 hours ago

@mbeacon or @owocki I would be greatful for any hints on how to populate ConversionRate with historic prices from created_on timestamp. I thought about implementing a loop over all bounties that would trigger an "get_historic_price" from polo/etherdelta against timestamp of created_on, but perhaps there is already a way that I did not discover yet e.g. running a test that will populate historic ConversionRate prices in the database.

@cryptomental
Copy link
Contributor

cryptomental commented Apr 4, 2018

I start to understand more and more. Just wanted to double check if ConversionRate object should point to 'web3_created' property, or 'created_on'.

"url": "/issue/networknt/react-schema-form/43",
        "created_on": "2018-04-03T07:39:23.651440-07:00",
        "modified_on": "2018-04-03T23:04:06.471557-07:00",
        "title": "conditionals",
        "web3_created": "2018-03-30T06:53:03-07:00",

...
        "expires_date": "2018-04-13T06:53:03-07:00",

Additionally, currently the cron job updates ConversionRate every three hours querying polo and etherdelta, and retention policy wipes data older than 7 days. If we are to access all old bounties data, the ConvertionRate should be preserved for all bounties web3_created timestamps to save hits to external APIs.

One simple idea is to iterate the bounties, keep the ConversionRate timestamps, wipe everything older out and merge the bounties ConversionRates back.

Currently convert_amount() from economy/utils always returns the latest price and my idea is to extend it with the optional timestamp argument:

def convert_amount(from_amount, from_currency, to_currency, timestamp=None):
    """Convert the provided amount to another current.

    Args:
        from_amount (float): The amount to be converted.
        from_currency (str): The currency identifier to convert from.
        to_currency (str): The currency identifier to convert to.
        timestamp (datetime): Timestamp of the conversion rate at given time. Latest if None.

    Returns:
        float: The amount in to_currency.

    """

The trick is though that ConversionRate is collected every three hours, and a created_on price will either mean price within 3 hour window before the web3_created or get_prices will have to be called when the bounty is submitted to have the "almost" exact value.

We will need to clarify those three issues for me to be able to implement the right approach.

  • should web3_created or created_on property be taken into account
  • is the approach with caching ConvertionRate objects after wiping the old data and merging it back fine
  • how to address 3 hour window of ConversionRate data

@owocki
Copy link
Contributor Author

owocki commented Apr 4, 2018

I start to understand more and more. Just wanted to double check if ConversionRate object should point to 'web3_created' property, or 'created_on'.

web3_created pls!

@owocki
Copy link
Contributor Author

owocki commented Apr 4, 2018

is the approach with caching ConvertionRate objects after wiping the old data and merging it back fine

yes i think so

how to address 3 hour window of ConversionRate data

i think that the 3 hour window is a fine degree of granularity

@vs77bb vs77bb added To Define and removed To Define labels Apr 5, 2018
@vs77bb vs77bb self-assigned this Apr 5, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
The bounty property and all methods were renamed to use value_in_usdt_now.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
The convert_amount method accepts optional timestamp datetime.datetime argument.
If the argument is not provided, the latest available ConversionRate is returned.

As a fallback if no data for a given timestamp is available, the latest available
rate is return.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
The value_in_usdt property returns value of the bounty using first available
ConversionRate after web3_created bounty creation time.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
Update the fields to reflect the current status. value_in_usdt holds bounty
USD value at web3_created timestamp.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 9, 2018
…t the time of the web3_created.

To stop fluctuating prices on bounties explorer, use ConversionRate at the web3_created bounty timestamp.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
The bounty property and all methods were renamed to use value_in_usdt_now.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
The convert_amount method accepts optional timestamp datetime.datetime argument.
If the argument is not provided, the latest available ConversionRate is returned.

As a fallback if no data for a given timestamp is available, the latest available
rate is return.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
The value_in_usdt property returns value of the bounty using first available
ConversionRate after web3_created bounty creation time.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
Update the fields to reflect the current status. value_in_usdt holds bounty
USD value at web3_created timestamp.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 10, 2018
…t the time of the web3_created.

To stop fluctuating prices on bounties explorer, use ConversionRate at the web3_created bounty timestamp.

Issue: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
get_prices iterates over all bounties in the DB and retrieves
ConversionRates from CryptoCompare if not yet available.

Add cryptocompare dependency to requirements.

Refs: gitcoinco#693, gitcoinco#491, gitcoinco#692
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
get_prices iterates over all bounties in the DB and retrieves
ConversionRates from CryptoCompare if not yet available.

Add cryptocompare dependency to requirements.

Refs: gitcoinco#693, gitcoinco#491, gitcoinco#692
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
The bounty property and all methods were renamed to use value_in_usdt_now.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
The convert_amount method accepts optional timestamp datetime.datetime argument.
If the argument is not provided, the latest available ConversionRate is returned.

As a fallback if no data for a given timestamp is available, the latest available
rate is return.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
The value_in_usdt property returns value of the bounty using first available
ConversionRate after web3_created bounty creation time.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
Update the fields to reflect the current status. value_in_usdt holds bounty
USD value at web3_created timestamp.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
…t the time of the web3_created.

To stop fluctuating prices on bounties explorer, use ConversionRate at the web3_created bounty timestamp.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
Verify that historic ConversionRate objects can be retrieved.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
…ateNotFoundError.

Fixed typo and made the name PEP8 compliant.

Refs: gitcoinco#693
cryptomental added a commit to cryptomental/web that referenced this issue Apr 11, 2018
get_prices iterates over all bounties in the DB and retrieves
ConversionRates from CryptoCompare if not yet available.

Add cryptocompare dependency to requirements.

Refs: gitcoinco#693, gitcoinco#491, gitcoinco#692
cryptomental added a commit to cryptomental/web that referenced this issue Apr 12, 2018
Added created_on property.

Refs: gitcoinco#693
@owocki
Copy link
Contributor Author

owocki commented Apr 12, 2018

#853 is live now

@gitcoinbot
Copy link
Member

The funding of 0.12 ETH (56.06 USD @ $467.2/ETH) attached to this issue has been approved & issued to @cryptomental.

@owocki owocki closed this as completed Apr 24, 2018
@gitcoinco gitcoinco deleted a comment May 26, 2018
@gitcoinco gitcoinco deleted a comment from darkdarkdragon Jun 7, 2018
@gitcoinco gitcoinco deleted a comment from darkdarkdragon Jun 7, 2018
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

No branches or pull requests

6 participants