Skip to content
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

Log the transaction hash in the module limit warning #76

Merged

Conversation

siladu
Copy link
Contributor

@siladu siladu commented Sep 18, 2024

tx hash might be obtainable from existing logs but it is currently only printed at TRACE

log.atTrace()
.setMessage(
"Result of simulation validation for tx with hash={}, isLocal={}, hasPriority={}, is {}, module line counts {}")
.addArgument(transaction::getHash)

@siladu siladu requested a review from fab-10 September 18, 2024 07:10
@siladu
Copy link
Contributor Author

siladu commented Sep 18, 2024

Leaving as draft as this is my first PR, I'm not certain what the testing process should be - any testing required beyond the units and acceptanceTests?

Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

LGTM, passing CI tests is enough, there is nothing else to do, you can merge

@siladu siladu marked this pull request as ready for review September 18, 2024 07:45
@siladu siladu merged commit 9c79cc0 into Consensys:main Sep 18, 2024
5 checks passed
@siladu siladu deleted the feature/simulationvalidator-log-tx-hash branch September 20, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants