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

Tweak tortoise vote calculation for testnet #2337

Closed
wants to merge 20 commits into from
Closed

Conversation

lrettig
Copy link
Member

@lrettig lrettig commented Mar 20, 2021

Motivation

Address an issue in #2298. Blocks can abstain now when hare fails, but empty layers aren't being finalized.

To do

  • Add tests

Changes

  • Count an abstain vote on the zero pattern as an explicit vote in support

@lrettig
Copy link
Member Author

lrettig commented Mar 20, 2021

bors try

bors bot added a commit that referenced this pull request Mar 20, 2021
@bors
Copy link

bors bot commented Mar 20, 2021

try

Build failed:

@lrettig lrettig changed the title Calculate net not absolute tortoise support Tweak tortoise vote calculation for testnet Mar 21, 2021
@lrettig lrettig marked this pull request as ready for review March 21, 2021 02:01
lrettig and others added 16 commits March 31, 2021 16:48
Fix merge conflicts, make lint happy

Add context, improve debugging of syncer

Context, logging for block listener, syncer

Add context to hare outgoing messages

Linter

More lint and test fixes

Fix merge conflicts

Pass context down to msg send on net

Improve net debug

Improve context and logging for outgoing messages

Improve context and logging for atx and block flow

Remove explicit context and reqID from ReportValidation, and read from
message context instead

Resolves merge conflicts from testnet-126 branch changes

Logging

Minor cleanup of atx logging

Bugfixes

Add log context to fetch request

Whitespace
Vote for whole layer if hare results missing

Lower tortoise thresholds for testnet

Increase verbosity

Vote for zero pattern instead

Update test to match modified logic

Improve log output

Stick with old newly good threshold

Minor test cleanup

Make test more readable

Update threshold and test

Update votes, test passes

Update second test

to match new thresholds

Cleanup threshold param

Cleanup test

Gofmt
## Motivation

It's confusing whether to click or not to click on the DevOps notes points. Re phrased them to make it clear.

Co-authored-by: Lane Rettig <lanerettig@gmail.com>
Co-authored-by: Anton Lerner <anton.lerner@gmail.com>
<!-- Please mention the issue fixed by this PR or detailed motivation -->
Closes #2068
<!-- `Closes #XXXX, closes #XXXX, ...` links mentioned issues to this PR and automatically closes them when this it's merged -->

<!-- Please describe in detail the changes made -->

1.  Added `SmesherID` and `Coinbase` fields to `struct Reward` for efficient look-up. Used this to complete the `AccountDataQuery` and `AccountDataStream` endpoints in the API code.
2. Changed the database index for storing Rewards to a key of the form `reward_<coinbase>_<smesherID>_<layerID>` because multiple SmesherIDs could be associated with the same Coinbase. Added a secondary key for Rewards lookups by SmesherID.
3. Changed the `accumulateRewards` function in `mesh/mesh.go` so to support handling multiple smesherIDs associated with a single Coinbase. Moved event reporting of the Rewards from the processor to the mesh code.
4. Added tests to `mesh/meshdb_test.go` to test changes made.
5. Implemented the API endpoints for `SmesherRewardStream` and `SmesherDataQuery` using the endpoints created.
6. Since we now have that there can be multiple `SmesherID`s associated with a particular `Coinbase`, changed the way the rewards are calculated. Now the `blockTotalReward` and `blockLayerReward` are multiplied by the number of blocks produced by a particular `Coinbase`-`SmesherID` pair.
<!-- Please specify how these changes were tested
(e.g. unit tests, manual testing, etc.) -->
Wrote multiple unit tests for both the APIs that I implemented, and the meshdb code that I changed.
<!-- This section should be removed when all items are complete -->
- [ ] Explain motivation or link existing issue(s)
- [ ] Test changes and document test plan
- [ ] Update documentation as needed

Co-authored-by: Lane Rettig <lanerettig@gmail.com>
## Motivation
<!-- Please mention the issue fixed by this PR or detailed motivation -->
Closes #2253 
<!-- `Closes #XXXX, closes #XXXX, ...` links mentioned issues to this PR and automatically closes them when this it's merged -->

## Changes
<!-- Please describe in detail the changes made -->
- Fix fatal error in `multi_node_sim`

## Test Plan
<!-- Please specify how these changes were tested 
(e.g. unit tests, manual testing, etc.) -->
1. Attempt to reproduce #2253 following steps from the PR description
2. There should be no fatal error
Remove a lingering reference to gitter from README

Co-authored-by: Anton Lerner <anton.lerner@gmail.com>
## Motivation
"CI status" for the develop branch is not really well-defined since we don't actually run any CI tasks on the develop branch. Bors runs them on staging, then merges to develop. If we set the status badge to develop, as it is now, it shows "no status." If we set it to staging, it'll show the status of the last bors run, which may have failed (and not been merged), which is misleading.

@noamnelke you were right, I concede :)
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
## Motivation
Fixes #2348

## Changes
- replace `t.FailNow()` in non-test goroutine with Panic

## Test Plan
N/A

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [ ] Update documentation as needed

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
## Motivation
Closes #2328

## Changes
For staging branch, i.e., for `bors merge` runs, use commithash rather than branch name (`staging`) as docker image tag to ensure that bors is running tests against the right code

## Test Plan
N/A
Fix typo

Spotter: @y0sher

Bugfix to pass tests

msgcon does not need its own closer. It's now closed from inside the UDP
wrapper. Otherwise, we get a panic on trying to close a closed channel.

Test that msgConn is closed on UDP conn eviction

Mocks the msgConn in the test. Added a very simple interface to make
this easier.

Another test fix

Need to init this channel manually in test

Return conn token if incoming conn fails

Spotter: @y0sher

Formatting

Add regression test for net max pending conn

Fix merge conflicts and make linter happy

Fix concurrency issue in test

Linter

Minor test cleanup

Add shutdowns to tests
lrettig added a commit that referenced this pull request Mar 31, 2021
Make linter happy

Count abstain on zero pattern as explicit support

Whitespace

Revert "Calculate net not absolute tortoise support"

This reverts commit 2a33fd1.

Bugfix in sendMessage

Bugfix

Bugfix

Fix test
lrettig added a commit that referenced this pull request Apr 1, 2021
Make linter happy

Count abstain on zero pattern as explicit support

Whitespace

Revert "Calculate net not absolute tortoise support"

This reverts commit 2a33fd1.

Bugfix in sendMessage

Bugfix

Bugfix

Fix test
@lrettig
Copy link
Member Author

lrettig commented Apr 26, 2021

This was included in the testnet branch in 632b6d6 and won't be included in develop since the ninja tortoise code has been deprecated. Closing.

@lrettig lrettig closed this Apr 26, 2021
@lrettig lrettig deleted the voting-patch-2 branch April 26, 2021 17:41
lrettig added a commit that referenced this pull request Jun 26, 2021
@lrettig lrettig mentioned this pull request Jul 2, 2021
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

Successfully merging this pull request may close these issues.

None yet

6 participants