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: implementing pip-15 #843

Merged
merged 36 commits into from
Dec 31, 2023
Merged

Conversation

amirvalhalla
Copy link
Member

@amirvalhalla amirvalhalla commented Dec 9, 2023

Description

This PR implements PIP-15.

@amirvalhalla amirvalhalla self-assigned this Dec 9, 2023
Copy link

codecov bot commented Dec 9, 2023

Codecov Report

Merging #843 (2e6d417) into main (6260b8b) will increase coverage by 0.09%.
The diff coverage is 85.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #843      +/-   ##
==========================================
+ Coverage   83.23%   83.32%   +0.09%     
==========================================
  Files         170      171       +1     
  Lines        8575     8673      +98     
==========================================
+ Hits         7137     7227      +90     
- Misses       1099     1104       +5     
- Partials      339      342       +3     

@amirvalhalla amirvalhalla changed the title feat: implement lru cache for account in store feat: implement pip-15 Dec 9, 2023
store/account.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
store/store.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
@b00f
Copy link
Collaborator

b00f commented Dec 14, 2023

Testing these change is super important. Please make sure all functionalities is tested properly

util/linkedlist/linkedlist.go Outdated Show resolved Hide resolved
util/linkedmap/linkedmap.go Outdated Show resolved Hide resolved
util/linkedmap/linkedmap.go Outdated Show resolved Hide resolved
util/linkedmap/linkedmap.go Show resolved Hide resolved
store/tx.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@b00f b00f left a comment

Choose a reason for hiding this comment

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

@amirvalhalla Thanks.
It is almost done, you did amazing Job.

config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config_test.go Show resolved Hide resolved
config/config_test.go Show resolved Hide resolved
node/node.go Show resolved Hide resolved
store/block.go Show resolved Hide resolved
store/config.go Outdated Show resolved Hide resolved
store/store.go Show resolved Hide resolved
util/linkedmap/linkedmap.go Outdated Show resolved Hide resolved
util/linkedmap/linkedmap.go Outdated Show resolved Hide resolved
util/tripleslice/tripleslice.go Outdated Show resolved Hide resolved
util/tripleslice/tripleslice.go Outdated Show resolved Hide resolved
util/tripleslice/tripleslice.go Outdated Show resolved Hide resolved
@b00f b00f changed the title feat: implement pip-15 feat: implementing pip-15 Dec 31, 2023
Copy link
Collaborator

@b00f b00f left a comment

Choose a reason for hiding this comment

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

Thanks @amirvalhalla. It is Almost done, just small changes before merging.

go.mod Show resolved Hide resolved
}

as.addrCache.Add(addr, acc)
return acc, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, I still feel that we need to review it again. We are returning an account pointer here, and it can be changed, which could potentially manipulate the cache.

store/tx.go Outdated Show resolved Hide resolved
@b00f b00f merged commit a5cc211 into pactus-project:main Dec 31, 2023
12 checks passed
Ja7ad added a commit to pactus-project/PIPs that referenced this pull request Jan 4, 2024
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