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(staking): Log Handler #160

Closed
wants to merge 82 commits into from
Closed

feat(staking): Log Handler #160

wants to merge 82 commits into from

Conversation

po-bera
Copy link
Contributor

@po-bera po-bera commented Feb 15, 2024

Summary by CodeRabbit

  • Bug Fixes

    • Corrected a typo in a function comment.
    • Added error handling for deposits processing.
    • Adjusted processing logs related to deposit and withdrawal events.
  • New Features

    • Added a new method ProcessLogs to the BuilderService interface for processing logs.
    • Introduced a logProcessor field for processing logs and a st field for staking service in the Service struct.
    • Added functions to set LogProcessor and StakingService of the Service.
    • Added a new function ProcessLogs in the Service struct for log processing and deposit persistence.
  • Refactor

    • Updated ProcessLogs method to handle errors during log processing and persist deposits.
  • Chores

    • Introduced new imports for log processing, callback handling, and staking services.

withdrawalCredentials,
)
// Cache the deposit to be pushed to the queue later in batch.
err = beaconState.CacheDeposit(deposit)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should put the cache on the service tbh. the State() should be really simple CRUD operations

return nil
}

// PersistDeposits commits the cached deposits to the queue
// and processes the queued deposits.
func (s *BeaconStore) PersistDeposits(n uint64) ([]*Deposit, error) {
func (s *BeaconStore) PersistDeposits(depositCache []*Deposit, n uint64) ([]*Deposit, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should move the check from within the store to the staking service, BeaconStore / State is supposed to be dumb CRUD logic.

@po-bera
Copy link
Contributor Author

po-bera commented Feb 19, 2024

@itsdevbear Is it branch good to merge? I added ValsetChangeProvider, which is separate from BeaconStateProvider. Don't know if you have a chance to look at it.

@po-bera po-bera requested a review from rustybera as a code owner February 19, 2024 22:23
@po-bera po-bera closed this Feb 20, 2024
@itsdevbear itsdevbear mentioned this pull request Feb 25, 2024
53 tasks
abi87 pushed a commit that referenced this pull request Feb 13, 2025
…load (#160)

* Update state_processor_payload.go

* only pass consensus time rather than full transitionctx

* rename txctx
abi87 pushed a commit that referenced this pull request Feb 13, 2025
…load (#160)

* Update state_processor_payload.go

* only pass consensus time rather than full transitionctx

* rename txctx
abi87 pushed a commit that referenced this pull request Feb 13, 2025
…load (#160)

* Update state_processor_payload.go

* only pass consensus time rather than full transitionctx

* rename txctx
abi87 pushed a commit that referenced this pull request Feb 14, 2025
…load (#160)

* Update state_processor_payload.go

* only pass consensus time rather than full transitionctx

* rename txctx
abi87 pushed a commit that referenced this pull request Feb 14, 2025
…load (#160)

* Update state_processor_payload.go

* only pass consensus time rather than full transitionctx

* rename txctx
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.

2 participants