-
Notifications
You must be signed in to change notification settings - Fork 6
Rollback feature #227
base: dev-rollback
Are you sure you want to change the base?
Rollback feature #227
Conversation
take changes from dev
# 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
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 { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used here?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
No description provided.