Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Rollback feature #227

Open
wants to merge 17 commits into
base: dev-rollback
Choose a base branch
from

Conversation

cristina-vasiu
Copy link
Contributor

No description provided.

Cristina and others added 13 commits October 3, 2019 16:48
# Conflicts:
#	src/main/java/net/helix/pendulum/TransactionValidator.java
#	src/main/java/net/helix/pendulum/controllers/RoundViewModel.java
#	src/main/java/net/helix/pendulum/crypto/Merkle.java
#	src/main/java/net/helix/pendulum/service/API.java
#	src/main/java/net/helix/pendulum/service/milestone/impl/MilestonePublisher.java
#	src/main/java/net/helix/pendulum/service/milestone/impl/MilestoneServiceImpl.java
#	src/main/java/net/helix/pendulum/service/validator/impl/ValidatorServiceImpl.java
@oracle58 oracle58 added the feature New feature or request label Nov 6, 2019
src/main/java/net/helix/pendulum/Pendulum.java Outdated Show resolved Hide resolved
return transactionViewModel.getTimestamp() < snapshotProvider.getInitialSnapshot().getTimestamp() && !snapshotProvider.getInitialSnapshot().hasSolidEntryPoint(transactionViewModel.getHash())
|| transactionViewModel.getTimestamp() > (System.currentTimeMillis() / 1000) + MAX_TIMESTAMP_FUTURE;
}
return transactionViewModel.getAttachmentTimestamp() < (snapshotProvider.getInitialSnapshot().getTimestamp())
|| transactionViewModel.getAttachmentTimestamp() > System.currentTimeMillis() + MAX_TIMESTAMP_FUTURE_MS;
}

private boolean isTransactionRequested(TransactionViewModel transactionViewModel) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

The business logic here is not clear. The methods seems to do much more than its name says -- in particular "cancel" the request etc. TransactionValidator at least as its name suggests, should not change the internal state, and handling the requested txs should be a responsibility of TransactionRequester

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were cases when transactions exist but also they were in requested list, this is why I have added an extra check.

Map<Hash, Integer> occurrences = new HashMap<>();
Map<Hash, Set<Hash>> milestoneHashes = new HashMap<>();

int quorum = (int)(config.getConfirmationQuorumPercentage() * BasePendulumConfig.Defaults.NUMBER_OF_ACTIVE_VALIDATORS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}


private static Set<Hash> splitMilestonesMerkleRoot(Tangle tangle, Hash expectedMilestoneMerkleRoot, TransactionViewModel milestoneTx) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

In view of our last discussion, does it make sense to simply pack milestone hashes into the bundle similar to the tips?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be a solution, but what is the purpose of virtual transactions then?

Copy link
Contributor

Choose a reason for hiding this comment

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

We will probably not use them at all, at least for now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants