refactor: committed tx may not have been executed #2525
Conversation
jstarry
force-pushed
the
refactor/committed-tx
branch
2 times, most recently
from
75b9c2a to
e8a5f54
Compare
jstarry
force-pushed
the
refactor/committed-tx
branch
from
e8a5f54 to
6487f28
Compare
|
The code looks good to me, but can you enlighten me re: the higher level context? Is this setting the stage for async execution? Charging fees for transactions we don't execute? Something else? |
Yup, charging fees for transactions that we didn't execute which is a step on the way to async execution. You can see how the code will be used here: https://github.com/anza-xyz/agave/pull/2425/files#diff-ed47b4a0198313377e091bb3957bbbc63d937805426d1b2b6de39d0a50d32a0cR3901 |
| @@ -51,7 +50,7 @@ impl ExecutedTransaction { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] | |||
| #[derive(Clone, Debug, Eq, PartialEq)] | |||
| pub struct TransactionExecutionDetails { | |||
There was a problem hiding this comment.
It would be nice if we could remove this struct entirely, now that we have struct CommittedTransaction which is almost identical.
I think we've got too many struct types :)
Problem
Refactor
CommittedTransactionstruct to allow representing transactions that were not executed but still committedSummary of Changes
execution_detailsfield inCommittedTransactionto remove the notion that those fields came from an executionTransactionCommitResultExtensions::transaction_resultmethod since it is easy to misuse given that it flattens processing and execution errorsTransactionCommitDetailsstruct for thebank_hash_detailsmodule that mirrorsCommittedTransactionso thatCommittedTransactionitself doesn't need to be serializableFixes #