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

Spentaddresses are persisted upon pruning #1258

Merged
merged 12 commits into from Jan 8, 2019

Conversation

Projects
None yet
5 participants
@GalRogozinski
Copy link
Member

GalRogozinski commented Jan 6, 2019

Description

This persists to the spent addresses db column family all the spent addresses from pruned transactions that were verified by the bundle validator.

We want the node to correctly verify spent addresses even after the corresponding transactions were pruned.

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)

How Has This Been Tested?

TBD... Blocked from merging until tested.

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

hmoog and others added some commits Jan 3, 2019

Feat: Write snapshot files to temp files first (#1256)
* Feat: write snapshot files to temp files first

* Refactor: rename now creates backup files (to force renamce over copy)
Revert "disable local-snapshots"
This reverts commit 793258d

@GalRogozinski GalRogozinski requested review from hmoog and alon-e Jan 6, 2019

@GalRogozinski GalRogozinski force-pushed the iotaledger:dev-localsnapshots branch from 5f1cf0c to 7072a7d Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from GalRogozinski Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from GalRogozinski Jan 6, 2019

import com.iota.iri.service.snapshot.Snapshot;
import com.iota.iri.service.snapshot.SnapshotException;
import com.iota.iri.service.snapshot.SnapshotProvider;
import com.iota.iri.service.snapshot.*;

This comment has been minimized.

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from codacy-bot Jan 6, 2019

@iotaledger iotaledger deleted a comment from GalRogozinski Jan 6, 2019

@iotaledger iotaledger deleted a comment from GalRogozinski Jan 6, 2019

@alon-e

alon-e approved these changes Jan 7, 2019

@@ -27,6 +27,11 @@ public TailFinderImpl(Tangle tangle) {
@Override
public Optional<Hash> findTail(Hash hash) throws Exception {
TransactionViewModel tx = TransactionViewModel.fromHash(tangle, hash);
return findTailFromTx(tx);

This comment has been minimized.

@alon-e

alon-e Jan 7, 2019

Member

it's probably worth changing in the other usages - but not part of the scope; open an issue?

This comment has been minimized.

@GalRogozinski

GalRogozinski Jan 7, 2019

Member

Since you commented and it is such an easy fix, I simply changed it in SpentAddressesServiceImpl

@@ -97,6 +99,11 @@ private MilestonePrunerJob(int startingIndex, int currentIndex, int targetIndex)
}
}

@Override
public void setSpentAddressesService(SpentAddressesService spentAddressesService) {
//Does nothing. Confirmed transactions spent are persisted during the local snapshot.

This comment has been minimized.

@alon-e

alon-e Jan 7, 2019

Member

this sounds a bit fragile. maybe only validate they are in the list? (i.e. query the spent addresses when you see a negative balance and throw is it fails?)

This comment has been minimized.

@GalRogozinski

GalRogozinski Jan 7, 2019

Member

Throwing an exception so it will stop pruning?
Actually it is a good idea

If there is a problem we want the operator to be annoyed and not be able to prune

GalRogozinski added some commits Jan 7, 2019

@GalRogozinski GalRogozinski merged commit 56a724f into iotaledger:dev-localsnapshots Jan 8, 2019

6 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
buildkite/iri-build-jar-prs Build #321 passed (14 minutes, 19 seconds)
Details
buildkite/iri-build-jar-prs/b9302ba8-9dcc-4bd5-ab72-22e08b96d1b4 Passed (7 minutes, 35 seconds)
Details
buildkite/iri-build-jar-prs/build-iri-oracle8 Passed (3 minutes, 57 seconds)
Details
buildkite/iri-build-jar-prs/pull-from-repo Passed (2 minutes, 43 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DyrellC

This comment has been minimized.

Copy link
Contributor

DyrellC commented Jan 9, 2019

Tested by setting up a node in testnet, and generating transactions and milestones until the node started taking snapshots. Then some value transactions were sent (while still spamming milestones and zero values) and I waited until they were close to pruning depth. I used checkConsistency periodically to ensure the transactions were pruning at the right depth, and made a simple IXI script to check the db for SpentAddress objects to verify that when the value transactions were pruned, there were persisted objects still present for the addresses. The spent addresses were added to the testnet.snapshot.spentAddresses file and I checked the consistency to make sure they weren't in the db anymore and I ran the IXI script to check if there were spentAddress objects in the tangle instance and there were 10 (the number of value transactions that I sent, and the number of addresses that were now in the spent addresses file).

Once this process was completed I stopped the spammers and restarted the node. When I rechecked the consistency and checked for persisted SpentAddress objects again, the transactions were pruned and the db SpentAddress objects were still there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment