[ecash-herald] Check peername API until updated
AbandonedPublic
Actions

Authored by bytesofman on Nov 24 2023, 14:56.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

In practice, the peerName API is not updated when ecash-herald calls it on block connected. Try 10 times, 3 seconds apart, to get the peerName from the API. Remove .filter method as you do not need to check every entry from API response, most likely only the first one or two.

Some refactoring required to make wait interval a param so that unit tests to not unnecessarily time out / take longer than needed. Add function param docs to functions where param is added.

Note: since we have the address of the winner from the block, we could avoid this wait interval if we had a peerName by address API.

Test Plan

npm test

Diff Detail

Repository

rABC Bitcoin ABC

Branch

troubleshoot-staker-peername

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25731
Build 51042: Build Diff	ecash-herald-tests
Build 51041: arc lint + arc unit

Event Timeline

bytesofman created this revision.Nov 24 2023, 14:56

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 24 2023, 14:56

bytesofman requested review of this revision.Nov 24 2023, 14:56

Harbormaster completed remote builds in B25730: Diff 43256.Nov 24 2023, 14:57

I simply don't understand the problem this is solving

fix comment, move wait interval to config

Harbormaster completed remote builds in B25731: Diff 43257.Nov 24 2023, 15:03

No more than 30s to wait for peername

apps/ecash-herald/config.js
11	testing overnight, no updates were observed sooner than 5s after block connected avg time was ~ 10s. Some blocks were > 20s.

Harbormaster completed remote builds in B25732: Diff 43259.Nov 24 2023, 15:05

Bump attempt count as 6 resulted in success for only 5 of 7 blocks

Harbormaster completed remote builds in B25736: Diff 43262.Nov 24 2023, 15:21

In D14850#333540, @Fabien wrote:

I simply don't understand the problem this is solving

ecash-herald is pinging https://avalanche.cash/api/recentstakingpayouts/XEC when a block is connected. However, this endpoint has not yet updated to include the staking reward winner for the block that just connected when the API call is made. It usually takes 5-30s for the endpoint to update.

So, the herald hits the endpoint, does not find the blockheight, and so does not add a peerName.

The best solution here is to get an endpoint that lists avalalnche peers by address and peerName, since we already know the address and don't really need to get the blockheight. But, the existing endpoint will suffice.

Another alt solution would be to hit the endpoint and check for any matching address. This would (usually) work on the first go, assuming the staking winner is in the endpoint's returned list of recent winners.

Would be less complicated to the app overall, could lose the whole wait interval and still get almost all of the peerNames by address matching. I figured though it would be easy enough to back out the wait interval if and when there is a different API / way to reference peerNames.

In D14850#333577, @bytesofman wrote:

In D14850#333540, @Fabien wrote:

I simply don't understand the problem this is solving

ecash-herald is pinging https://avalanche.cash/api/recentstakingpayouts/XEC when a block is connected. However, this endpoint has not yet updated to include the staking reward winner for the block that just connected when the API call is made. It usually takes 5-30s for the endpoint to update.

So, the herald hits the endpoint, does not find the blockheight, and so does not add a peerName.

So far I got it.

The best solution here is to get an endpoint that lists avalalnche peers by address and peerName, since we already know the address and don't really need to get the blockheight. But, the existing endpoint will suffice.

Another alt solution would be to hit the endpoint and check for any matching address. This would (usually) work on the first go, assuming the staking winner is in the endpoint's returned list of recent winners.

Would be less complicated to the app overall, could lose the whole wait interval and still get almost all of the peerNames by address matching. I figured though it would be easy enough to back out the wait interval if and when there is a different API / way to reference peerNames.

And none of this makes any sense to me. The only conclusion is that you didn't identify the problem correctly, at least not the root cause. You should go back to the drawing board and actually fix the issue rather than adding a bandaid over a flaky design.

This revision now requires changes to proceed.Nov 24 2023, 21:49

alt approaches are available with lower complexity