txn emitter - add more info on failed txn #15756
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⏱️ 2h 16m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
504
to
514
| let failured_txn_info = format!( | ||
| "due to {:?}, for account {}, max gas {}, payload {}", | ||
| failure, | ||
| first_failed_txn.sender(), | ||
| first_failed_txn.max_gas_amount(), | ||
| payload, | ||
| ); |
There was a problem hiding this comment.
The variable failured_txn_info contains a typo - "failured" is not a valid English word. Consider renaming to failed_txn_info for better readability. This change would need to be made in both the declaration and where it's used on line 536.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
igor-aptos
force-pushed
the
igor/emitter_txn_info
branch
from
2e28394 to
556c7c5
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
igor-aptos
force-pushed
the
igor/emitter_txn_info
branch
from
556c7c5 to
5f0b217
Compare
Comment on lines
168
to
178
| let payload = match first_failed_txn.payload() { | ||
| Script(_) => "script".to_string(), | ||
| ModuleBundle(_) => "module_bundle".to_string(), | ||
| EntryFunction(entry_function) => format!( | ||
| "entry {}::{}", | ||
| entry_function.module(), | ||
| entry_function.function() | ||
| ), | ||
| Multisig(_) => "multisig".to_string(), | ||
| }; |
There was a problem hiding this comment.
The variable first_failed_txn is undefined in this scope - this function receives a parameter named txn. The match statement should use txn.payload() instead of first_failed_txn.payload().
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
igor-aptos
force-pushed
the
igor/emitter_txn_info
branch
2 times, most recently
from
9cb2c90 to
b71ac0e
Compare
This comment has been minimized.
This comment has been minimized.
❌ Forge suite
|
runtian-zhou
approved these changes
Jan 16, 2025
| ) { | ||
| let handle = permissioned_signer::create_permissioned_handle(source); | ||
| let permissioned_signer = permissioned_signer::signer_from_permissioned_handle(&handle); | ||
| // public entry fun transfer_permissioned( |
There was a problem hiding this comment.
Are we deleting this because the forge upgrade didn't have the framework code locally just yet?
There was a problem hiding this comment.
no - because framework upgrade test issues transactions from framework_usecases module to test framework upgrade - but framework from previous release doesn't have it - so test fails
msmouse
approved these changes
Jan 16, 2025
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
igor-aptos
force-pushed
the
igor/emitter_txn_info
branch
from
b71ac0e to
58b0c94
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
and remove test that uses new code - which fails framework upgrade.
Description
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist