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

Fix memory #530

Merged
merged 3 commits into from Jul 25, 2019

Conversation

@a-bezrukov
Copy link
Contributor

commented Jul 22, 2019

No description provided.

Andrey added some commits Jul 20, 2019

Andrey
Andrey
@levonpetrosyan93
Copy link
Collaborator

left a comment

LGTM

@reubenyap reubenyap added this to In progress in Zcoin Core via automation Jul 22, 2019

@reubenyap reubenyap added this to the v0.13.8.3 milestone Jul 22, 2019

@reubenyap reubenyap added the bug label Jul 22, 2019

src/txmempool.cpp Outdated Show resolved Hide resolved
src/zerocoin_v3.cpp Show resolved Hide resolved
src/zerocoin_v3.cpp Show resolved Hide resolved

Zcoin Core automation moved this from In progress to Needs review Jul 23, 2019

src/txmempool.cpp Outdated Show resolved Hide resolved
mintedPubCoins.erase(iter);
CheckSurgeCondition(iter->second.coinGroupId, iter->second.denomination);
CheckSurgeCondition(tmpMintInfo.coinGroupId, tmpMintInfo.denomination);

This comment has been minimized.

Copy link
@psolstice

psolstice Jul 23, 2019

Contributor

If removal of a mint creates surge condition it would mean some problem other than surge (e.g. incorrect sigma state). Probably it deserves an assert. The only thing that can lead to the surge is new spend

src/zerocoin_v3.cpp Show resolved Hide resolved
usedCoinSerials.erase(iter);
CheckSurgeCondition(iter->second.coinGroupId, iter->second.denomination);
CheckSurgeCondition(tmpSpendInfo.coinGroupId, tmpSpendInfo.denomination);

This comment has been minimized.

Copy link
@psolstice

psolstice Jul 23, 2019

Contributor

Removal of spend can never lead to surge condition

This comment has been minimized.

Copy link
@levonpetrosyan93

levonpetrosyan93 Jul 23, 2019

Collaborator

But it can remove the condition.

This comment has been minimized.

Copy link
@psolstice

psolstice Jul 24, 2019

Contributor

Point taken but just to prevent forks when surging

@a-bezrukov a-bezrukov dismissed stale reviews from psolstice and levonpetrosyan93 via 4f7e055 Jul 24, 2019

Zcoin Core automation moved this from Needs review to Reviewer approved Jul 25, 2019

@ultimaweapon ultimaweapon merged commit f5c2ea5 into master Jul 25, 2019

1 of 3 checks passed

LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details

Zcoin Core automation moved this from Reviewer approved to Done Jul 25, 2019

@ultimaweapon ultimaweapon deleted the fix_memory branch Jul 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.