geyser: update to ReplicaAccountInfoV4#6520
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6520 +/- ##
=======================================
Coverage 82.8% 82.8%
=======================================
Files 849 849
Lines 379159 379164 +5
=======================================
+ Hits 314069 314146 +77
+ Misses 65090 65018 -72 🚀 New features to boost your workflow:
|
|
Why don't use #5114 and leave only runtime updates in the account update? |
|
can we also include is_simple_vote_transaction flag? Currently it is possible to filter out early geyser account updates related to vote transactions which will not be the case if we remove a reference to SanitizedTransaction |
jstarry
force-pushed
the
geyser-account-v4
branch
from
5bec21e to
464abcc
Compare
@fanatid I think it we can still leave all account updates (both tx and runtime) but strip down the tx context
@4r33x Good call, I'm helping to get #5114 in soon, but at the same time I don't want to block this change too long since it is necessary for some other work I'm doing.
|
|
|
||
| /// The updater of the account, which can be either a transaction or the | ||
| /// runtime. | ||
| pub updater: ReplicaAccountUpdater<'a>, |
There was a problem hiding this comment.
I am concerned with such changes removing fields for the transaction. For plugins relying on this, they would need to make large changes to look up. ReplicaTransactionInfoV3 already exposes SanitizedTransaction so this change little facts on the encapsulation side. Also, SanitizedTransaction is not exactly internal as it is declared in the SDK. I think we should address them comprehensively in a new breaking change for interfaces.
There was a problem hiding this comment.
ReplicaTransactionInfoV3 already exposes SanitizedTransaction so this change little facts on the encapsulation side
I'm not sure what you mean. You reviewed #6515 which removes SanitizedTransaction from ReplicaTransactionInfoV3 in favor of using VersionedTransaction.
Also, SanitizedTransaction is not exactly internal as it is declared in the SDK.
Unfortunately many internal types have been added to the SDK and we are trying to clean that up. SanitizedTransaction cannot be used for transactions that fail account resolution so we need to migrate away from using that type.
I think we should address them comprehensively in a new breaking change for interfaces.
Yes, they will need to migrate when they upgrade to agave v3 which is an appropriate time to make such breaking changes.
There was a problem hiding this comment.
I meant removing the transaction and forcing the current plugin to the additional lookup. I am okay to change to VersionedTransaction here as well. I would be easier for the plugin to change to integrate the new changes. Otherwise, they would have to cache the transaction changes or do additional lookup in the database. Some plugin may not even subscribe to the transaction interface and solely depends on this interface.
There was a problem hiding this comment.
Yeah I'm ok with switching to VersionedTransaction here but I don't think it's the best idea because unlike transaction updates, we wouldn't have any transaction metadata like resolved lookup table addresses
There was a problem hiding this comment.
You meant it did not have TransactionStatusMeta?
There was a problem hiding this comment.
The earlier account updater interface did not have TransactionStatusMeta either. Can one still make use of the message in it?
|
In the Yellowstone Dragon's Mouth plugin/proto only signature is exposed anyway. |
|
This pull request is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
7 participants
Problem
SanitizedTransactionis a runtime type that should have never been exposed to end users via sdk or geyser. Also, in order to implement solana-foundation/solana-improvement-documents#192, accounts will be able to be updated (specifically fee payer and nonce accounts) by transactions which cannot be represented as SanitizedTransaction's due to address lookup failures.Summary of Changes
Effectively revert solana-labs#30189 by removing the transaction reference from account updates. Instead, provide the transaction message hash and signature and make the consumer lookup the transaction OR have them subscribe to transactions updates with pre/post account recording enabled (#5114)
Fixes #