Issue: Early withdrawal claims #23
Conversation
|
This fix requires a //Edit: to avoid unintended forking, the added check should be surrounded with a hard fork check. See https://github.com/cryptonomex/graphene/pull/617/files for example. |
|
Thank you for the example case. The fix and tests have been updated to consider a new fork time. The fork time has been set to February 1, 2017. |
|
@vikramrajkumar please review. |
libraries/app/database_api.cpp
Outdated
| @@ -1018,7 +1018,7 @@ market_ticker database_api_impl::get_ticker( const string& base, const string& q | |||
| result.lowest_ask = 0; | |||
| result.highest_bid = 0; | |||
|
|
|||
| auto price_to_real = [&]( const share_type a, int p ) { return double( a.value ) / pow( 10, p ); }; | |||
| // auto price_to_real = [&]( const share_type a, int p ) { return double( a.value ) / pow( 10, p ); }; | |||
There was a problem hiding this comment.
This change looks unrelated to the issue.
There was a problem hiding this comment.
That is correct. I noticed that it was an unused variable. Do you prefer that I remove it from the commit?
There was a problem hiding this comment.
Please rebase on develop and see if this change remains. In that case I'd be OK with it, but instead of commenting out the line just remove it completely.
| * @param current_time Current time | ||
| * @return A description of the current period | ||
| */ | ||
| withdrawal_period_descriptor current_period(fc::time_point_sec current_time) const { |
There was a problem hiding this comment.
AFAICS the withdrawal_period_descriptor and this method are only used in unit tests. IMO they shouldn't become part of the main code in that case.
There was a problem hiding this comment.
Fair point, although I have this object being used for code (that is not yet committed) which is related to informing a user/client software about what can still be withdrawn during any particular period.
Do you suggest that I:
(1) temporarily move this code out of the main code, and then later
(2) move this class into the main code when committing my future code?
Partial fix for issue 303
* new pull for issue 298 * Revert "finish sentence in readme" This reverts commit bf76c59. * Revert "Add api-access console log to node startup when present" This reverts commit 80d81ac. * add support for start=0 * changes to rvalues * Update application.cpp * Update application.cpp * Update application.cpp * Update README.md
Fix bitshares#367 - use unique options list for parsing config file
|
@TheTaconator do you think you can add the requested modifications within the next couple of days? |
database api changes in witnesses, committee members and workers calls.
instead of <= 100 in get_full_accounts.
|
Also "this branch has conflicts that must be resolved". Need to rebase. |
change distance() in get_full_accounts to size()
|
@pmconrad apologies, but due to some other deadlines I will not be able to make the changes in time. I might need another 7 days. I have no objections about proceeding without this fix for now. |
…ro_test Add a test that reproduce something-for-nothing fill bug
…ior before and after a fork time for the proposed resolution
TheTaconator
force-pushed
the
fix_withdrawal_claim_too_early
branch
from
e521e05
to
8a70136
Compare
|
Closing here, continued in #386 |
abitmore
modified the milestones:
Hardfork - Bugs ,
Hardfork - 201711 - BSIP 018 and etc
Fix of bug that permits withdrawal claims before the first withdrawal period