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

Track added at in txpool #1048

Merged

Conversation

rojotek
Copy link
Member

@rojotek rojotek commented Mar 5, 2019

Store the instant that a pending transaction is added to the txpool, and expose this out through the api.

@@ -109,7 +110,7 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() {
new CliqueBlockCreator(
coinbase,
parent -> extraData.encode(),
new PendingTransactions(5),
new PendingTransactions(5, Clock.systemUTC()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the fixed clock in tests please? Real-time in tests comes back to bite you. Might need a util method somewhere to make it easy since Clock.fixed requires specifying a time and we generally don't care what time is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a TestClock with a fixed() static method.

@@ -147,7 +147,7 @@ private MainnetPantheonController(

final TransactionPool transactionPool =
TransactionPoolFactory.createTransactionPool(
protocolSchedule, protocolContext, ethProtocolManager.ethContext());
protocolSchedule, protocolContext, ethProtocolManager.ethContext(), Clock.systemUTC());
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good if we could only create one clock instance in the controller (there's a second one being passed to DefaultBlockScheduler.

Copy link
Member Author

Choose a reason for hiding this comment

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

done -- we now have a single call to Clock.systemUTC() in the codebase.

}

@JsonGetter(value = "hash")
public String getHash() {
return hash;
}

@JsonGetter(value = "addedToPoolAt")
public long getAddedToPoolAt() {
return addedToPoolAt.toEpochMilli();
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed this should be ISO8601 format rather than epoch millis.

Copy link
Member Author

Choose a reason for hiding this comment

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

cool -- have used toString on the instant which returns an ISO8601 string.

@rojotek rojotek force-pushed the track-added-at-in-transaction-pool branch from 120a920 to 8a3aedd Compare March 6, 2019 19:24
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM

TestClock class added with a factory method for a fixed clock.
@rojotek rojotek force-pushed the track-added-at-in-transaction-pool branch from 4c0e8d9 to 4e177af Compare March 7, 2019 11:29
@rojotek rojotek merged commit 4ad79be into PegaSysEng:master Mar 7, 2019
@rojotek rojotek deleted the track-added-at-in-transaction-pool branch March 7, 2019 18:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants