-
Notifications
You must be signed in to change notification settings - Fork 878
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
Improve newPayload and FCU logs #7961
Changes from 26 commits
9e558e5
ca3962d
b8a84da
2aaf354
3fff1c5
454c59d
0d82b32
eb0321b
1755514
8ff711c
88353ac
ad6eb05
04d6521
bf26d50
5d3786d
1dbc590
812bafe
ab86b6c
a18e6d5
6be3e28
05d5cde
6b9672b
297893d
20f714e
01cf738
68a056c
a7534e7
50e8f0b
7415e38
e1d2395
4f58822
0c903c7
07ce1a6
69d4564
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,8 @@ public enum Status { | |
|
||
private final Bytes output; | ||
|
||
private Optional<Boolean> isProcessedInParallel = Optional.empty(); | ||
|
||
private final ValidationResult<TransactionInvalidReason> validationResult; | ||
private final Optional<Bytes> revertReason; | ||
|
||
|
@@ -194,6 +196,25 @@ public ValidationResult<TransactionInvalidReason> getValidationResult() { | |
return validationResult; | ||
} | ||
|
||
/** | ||
* Set isProcessedInParallel to the value in parameter | ||
* | ||
* @param isProcessedInParallel new value of isProcessedInParallel | ||
*/ | ||
public void setIsProcessedInParallel(final Optional<Boolean> isProcessedInParallel) { | ||
this.isProcessedInParallel = isProcessedInParallel; | ||
} | ||
Comment on lines
+204
to
+206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we avoid to make this class mutable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that currently, when a transaction executes, it doesn't know itself if it si being executed in parallel or not, so the instance of TransactionProcessingResult is not aware if it was parallel or not. This information is only accessible later on. that's why we need to make the result mutable at this level only. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok I see, fine for me, but maybe this is a sign that some refactor is needed, to postponed the creation of the result, of the data should be kept elsewhere |
||
|
||
/** | ||
* Returns a flag that indicates if the transaction was executed in parallel | ||
* | ||
* @return Optional of Boolean, the value of the boolean is true if the transaction was executed | ||
* in parallel | ||
*/ | ||
public Optional<Boolean> getIsProcessedInParallel() { | ||
return isProcessedInParallel; | ||
} | ||
|
||
/** | ||
* Returns the reason why a transaction was reverted (if applicable). | ||
* | ||
|
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.
same comment about making etherscan debugging more difficult, less important for FCU though I think since can cross ref with Imported log
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 same as for newPayload call.