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

feat: added vote collector, refactored broadcast #36

Merged
merged 25 commits into from
May 4, 2023

Conversation

randyahx
Copy link
Contributor

@randyahx randyahx commented Mar 24, 2023

Description

added a vote collector that queries the blockchain for votes using eventhash and stores them for 2/3 consensus.
refactored broadcast to collate votes using eventhash instead of challengeid, broadcast votes periodically until expired height
refactored broadcaster, collator, submitter to use for loop instead of goroutine to solve performance issues
refactored workflow for collector, broadcaster, collator, submitter
added db wiper to periodically clear db

Rationale

services could not sync their votes to submit a msgattest

Example

none

Changes

Notable changes:
vote collector
vote broadcast
models, dao
data providers

db/model/vote.go Outdated Show resolved Hide resolved
vote/const.go Outdated Show resolved Hide resolved
forcodedancing

This comment was marked as resolved.

db/model/vote.go Outdated Show resolved Hide resolved
@forcodedancing
Copy link
Collaborator

Please check preCheck logics: currently, we allow challenge with small id attested after a bigger id.
For example, if challenge 90 attested, we can still attest 40 if it is not expired (expiredHeight).

db/dao/vote_dao.go Outdated Show resolved Hide resolved
db/model/vote.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app_test.go Outdated Show resolved Hide resolved
common/const.go Outdated Show resolved Hide resolved
config/config.json Outdated Show resolved Hide resolved
db/dao/event_dao_test.go Outdated Show resolved Hide resolved
db/model/event.go Outdated Show resolved Hide resolved
db/model/event.go Outdated Show resolved Hide resolved
@@ -34,119 +36,108 @@ func NewTxSubmitter(cfg *config.Config, executor *executor.Executor, daoManager
}

func (s *TxSubmitter) SubmitTransactionLoop() {
s.cachedEventHash = make(map[uint64][]byte, math.MaxInt)
submitAttempt := 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we discussed, the logic will change a bit after the in-turn challenger on chain.

"strings"
"sync"
"time"

"github.com/panjf2000/ants/v2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are some TODO: revert in this file. Could you check and fix them?

func (h *DataHandler) FetchEventsForSelfVote() ([]*model.Event, error) {
events, err := h.daoManager.GetEarliestEventsByStatusAndAfter(model.Verified, batchSize, h.lastIdForSelfVote)
func (h *DataHandler) FetchEventsForSelfVote(currentHeight uint64) ([]*model.Event, error) {
// TODO: Revert this
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check this.

voteBroadcaster: voteBroadcaster,
voteCollator: voteCollator,
txSubmitter: txSubmitter,
dbWiper: dbWiper,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need wiper? Sometimes we need to review or query historical data. We may not need it now.

@forcodedancing forcodedancing force-pushed the feat/refactor-collector-broadcast branch from 12e33b0 to f819cd7 Compare April 19, 2023 12:28
@randyahx randyahx merged commit cee60fc into develop May 4, 2023
3 checks passed
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

2 participants