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

Test that pools have non-myopic member rewards #1783

Merged
merged 4 commits into from
Jun 30, 2020

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jun 22, 2020

Issue Number

#1276

Overview

  • Test that pools have non-zero non-myopic member rewards in GET /stake-pools
  • Test that pools are sorted by descending rewards
  • Add MsgWillQueryRewardsForStake trace

Comments

@Anviking Anviking self-assigned this Jun 23, 2020
@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from 8ff53f2 to 636d4e4 Compare June 23, 2020 19:57
@Anviking Anviking changed the title Enable "Rewards accumulate and stop" test Test that we see non-myopic member rewards, and actual rewards alike Jun 23, 2020
@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from 636d4e4 to 4c73df1 Compare June 26, 2020 14:40
@Anviking Anviking changed the title Test that we see non-myopic member rewards, and actual rewards alike Test that pools have non-myopic member rewards Jun 26, 2020
@Anviking Anviking force-pushed the anviking/enable-rewards-test branch 2 times, most recently from 0223947 to ec1fc6c Compare June 29, 2020 13:29
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 29, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 29, 2020

try

Build failed

@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from ec1fc6c to 03ae72b Compare June 29, 2020 15:54
@Anviking Anviking changed the base branch from master to rvl/adp-302/db-reward-balance June 29, 2020 15:59
@Anviking
Copy link
Member Author

bors try

@Anviking Anviking marked this pull request as ready for review June 29, 2020 16:02
iohk-bors bot added a commit that referenced this pull request Jun 29, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 29, 2020

try

Build failed

@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from 17b7799 to 30870fc Compare June 29, 2020 17:07
Anviking added 2 commits June 29, 2020 19:08
If the ranking is weird, we can know what stake-value was used.
Fixup: Add todo about STAKE_POOLS_LIST_01 not passing in isolation

Also test that pools are sorted
@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from 30870fc to 4f436a6 Compare June 29, 2020 17:09
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 29, 2020
@@ -790,6 +792,8 @@ instance ToText NetworkLayerLog where
[ "Destroying cursor connection at"
, T.pack (show threadId)
]
MsgWillQueryRewardsForStake c ->
"Will query non-myopic rewards using the stake " <> pretty c
Copy link
Member Author

@Anviking Anviking Jun 29, 2020

Choose a reason for hiding this comment

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

Is the balance private and not to be logged? (not necessarily the user's balance, but likely correlated)

Copy link
Member

Choose a reason for hiding this comment

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

Nope, fine. Balances are already logged as part of the wallet. Current logs aren't much private.

@Anviking Anviking requested a review from KtorZ June 29, 2020 17:16
@Anviking Anviking added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Jun 29, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 29, 2020

try

Build failed

Base automatically changed from rvl/adp-302/db-reward-balance to master June 30, 2020 09:36
@KtorZ
Copy link
Member

KtorZ commented Jun 30, 2020

(trying the branch locally)

@KtorZ
Copy link
Member

KtorZ commented Jun 30, 2020

bors r+

@KtorZ
Copy link
Member

KtorZ commented Jun 30, 2020

Locally, the test pass when run in isolation. But it fails when run with the entire test-suite. One of two conditions is not verified.

@Anviking
Copy link
Member Author

It seems the pools all have the same non-myopic member rewards. Perhaps leaving them running for long removes the noise.

I tried giving one pool twice the amount of pledge/stake, and it seemed to work. But perhaps giving each pool different costs, or margins would be neater 🤔

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 30, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 30, 2020

try

Build failed


1) API Specifications STAKE_POOLS_QUIT_02 - Passphrase must be correct to quit
--
  | [cardano-wallet.integration:Notice:785] [2020-06-30 16:48:18.28 UTC] bft [cardano-node.68006] Terminating child process
  | expected: Status {statusCode = 202, statusMessage = "Accepted"}
  | [cardano-wallet.integration:Notice:866] [2020-06-30 16:48:18.28 UTC] pool-1 [cardano-node.68243] Terminating child process
  | but got: Status {statusCode = 404, statusMessage = "Not Found"}
  |  
  | from the following response: Left (DecodeFailure "Error in $: parsing Cardano.Wallet.Api.Types.ApiTransaction(ApiTransaction) failed, key \"id\" not found: Response {responseStatus = Status {statusCode = 404, statusMessage = \"Not Found\"}, responseVersion = HTTP/1.1, responseHeaders = [(\"Transfer-Encoding\",\"chunked\"),(\"Date\",\"Tue, 30 Jun 2020 16:41:51 GMT\"),(\"Server\",\"Warp/3.3.5\")], responseBody = \"{\\\"code\\\":\\\"no_such_pool\\\",\\\"message\\\":\\\"I couldn't find any stake pool with the given id: ec28f33dcbe6d6400a1e5e339bd0647c0973ca6c0cf9c2bbe6838dc6\\\"}\", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}")
  |  
  | To rerun use: --match "/API Specifications/STAKE_POOLS_QUIT_02 - Passphrase must be correct to quit/"
  |  
  | src/Test/Integration/Scenario/API/Shelley/StakePools.hs:187:5:
  | 2) API Specifications STAKE_POOLS_JOIN_01 - Can rejoin another stakepool
  | uncaught exception: RequestException
  | DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"
  |  
  | To rerun use: --match "/API Specifications/STAKE_POOLS_JOIN_01 - Can rejoin another stakepool/"


@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 30, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 30, 2020

try

Build failed

Such that we can test that pools actually are ordered by descending
rewards.
@Anviking Anviking force-pushed the anviking/enable-rewards-test branch from 3bf90ac to 367b626 Compare June 30, 2020 18:21
@Anviking
Copy link
Member Author

Anviking commented Jun 30, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Jun 30, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 30, 2020

try

Build failed

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 30, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 30, 2020

@Anviking
Copy link
Member Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 30, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit c493db9 into master Jun 30, 2020
@iohk-bors iohk-bors bot deleted the anviking/enable-rewards-test branch June 30, 2020 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants