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.

Sign up for GitHub

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

Jump to bottom

feat(pool): handle UREP-020 and track reputation of unstaked entities #522

Merged
merged 6 commits into from
Jan 29, 2024
Merged

feat(pool): handle UREP-020 and track reputation of unstaked entities #522

merged 6 commits into from
Jan 29, 2024

Conversation

alex-miao
Copy link
Collaborator

@alex-miao alex-miao commented Dec 4, 2023 •
edited
Loading

Closes #454, closes #354

Proposed Changes

  • Track reputation of all entities
  • Move SAME_SENDER_MEMPOOL_COUNT check out from PoolInner into UoPool
  • Do UREP-020 check in the same pool read lock as SAME_SENDER_MEMPOOL_COUNT check
  • Fix bug where we increment instead of decrement the reputation of an entity for an unmined transaction

@codecov Codecov
Copy link

codecov bot commented Dec 4, 2023 •
edited
Loading

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (5559d8f) 57.06% compared to head (e1f77bb) 57.15%.

Additional details and impacted files

Impacted file tree graph

Files Coverage Δ
crates/pool/src/mempool/error.rs 76.19% <ø> (ø)
crates/pool/src/mempool/pool.rs 93.61% <100.00%> (-0.12%) ⬇️
crates/sim/src/simulation/simulation.rs 77.21% <100.00%> (+0.18%) ⬆️
crates/pool/src/mempool/reputation.rs 76.82% <0.00%> (ø)
bin/rundler/src/cli/pool.rs 0.00% <0.00%> (ø)
crates/pool/src/server/remote/error.rs 16.84% <0.00%> (ø)
crates/pool/src/mempool/mod.rs 87.36% <80.55%> (-9.07%) ⬇️
crates/pool/src/mempool/uo_pool.rs 88.06% <75.00%> (+0.27%) ⬆️

... and 1 file with indirect coverage changes

Flag Coverage Δ
unit-tests 57.15% <81.75%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
rundler binary 0.00% <0.00%> (ø)
builder 50.09% <ø> (ø)
dev 0.00% <ø> (ø)
pool 63.95% <82.44%> (+0.16%) ⬆️
provider 0.91% <ø> (ø)
rpc 37.42% <ø> (ø)
sim 83.51% <100.00%> (+0.03%) ⬆️
tasks ∅ <ø> (∅)
types 90.25% <ø> (ø)
utils 14.96% <ø> (ø)

@alex-miao alex-miao force-pushed the alex/urep-020 branch 2 times, most recently from 003349f to ba528c8 Compare December 4, 2023 21:32
@alex-miao alex-miao marked this pull request as draft December 5, 2023 16:14
@alex-miao alex-miao force-pushed the alex/urep-020 branch 3 times, most recently from d679944 to c70cee8 Compare December 5, 2023 21:18
@github-actions github-actions bot added stale and removed stale labels Jan 5, 2024
@alex-miao alex-miao marked this pull request as ready for review January 16, 2024 18:05
dancoombs
dancoombs reviewed Jan 19, 2024
View reviewed changes
Copy link
Collaborator

@dancoombs dancoombs left a comment

Choose a reason for hiding this comment

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

Lets make sure this is as close to compliant with the latest ERC-7562, while still passing all tests on the commit we've pegged to.

If there are changes in ERC-7562 that cause failures, lets capture them as tickets so we know to update them when we move to a new spec test commit.

One more small nit on the PR to fix up the entities stuff.

@0xfourzerofour
Copy link
Collaborator

Do these changes fix those errors we spoke about in the spec tests?

@alex-miao alex-miao force-pushed the alex/urep-020 branch 2 times, most recently from c2c2f98 to 0b881ff Compare January 25, 2024 06:37
dancoombs
dancoombs requested changes Jan 26, 2024
View reviewed changes
Copy link
Collaborator

@dancoombs dancoombs left a comment

Choose a reason for hiding this comment

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

One last comment and then this is good to go. Can you squash to a single commit?

@0xfourzerofour
Copy link
Collaborator

@dancoombs I enabled squash and merge after that chat the other day, will use the PR title as the commit message

@alex-miao alex-miao merged commit cc23515 into main Jan 29, 2024
7 checks passed
@alex-miao alex-miao deleted the alex/urep-020 branch January 29, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dancoombs dancoombs dancoombs approved these changes

@dphilipson dphilipson Awaiting requested review from dphilipson dphilipson is a code owner

@0xfourzerofour 0xfourzerofour Awaiting requested review from 0xfourzerofour 0xfourzerofour is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[spec] UREP-20 pool: Still track reputation of non-stake-allowed entities.
3 participants
@alex-miao @0xfourzerofour @dancoombs