feat: rework data formats used in bubblegum transaction processing #422
Conversation
armyhaylenko
force-pushed
the
feature/MTG-1298-data-formats
branch
5 times, most recently
from
f20572a to
ffea676
Compare
armyhaylenko
force-pushed
the
feature/MTG-1298-data-formats
branch
2 times, most recently
from
0d559c6 to
61036b1
Compare
skip-checks: true
armyhaylenko
force-pushed
the
feature/MTG-1298-data-formats
branch
from
8918144 to
99f3122
Compare
| .unwrap_or_default() | ||
| .into_iter() | ||
| .filter_map(|t| { | ||
| decode_encoded_transaction_with_status_meta(t).and_then(|t| { |
There was a problem hiding this comment.
Just some thoughts, no immediate action needed. The call above has the encoding options and maybe the binary one will be easier to handle (cpu-wise, not coding wise)
grpc/Cargo.toml
Outdated
| tracing = { workspace = true } | ||
| rocks-db = { path = "../rocks-db" } |
There was a problem hiding this comment.
It's a bit ugly in terms of dependencies as both rocks and grpc are concrete implementations. The encoding on the wire shouldn't really depend on the encoding used on storage layer. As grpc layer will communicate in terms of entities, and not raw bytes. I know Rust makes any attempts of clean arch a nightmare, but this one seems reasonable solid - specify the encoding/decoding directly when transforming the entities to grpc types (thus the bincode above makes sense)
All the other usages of RawBlock::decode in the PR are 100% valid
entities/src/utils.rs
Outdated
There was a problem hiding this comment.
Let's rename it to solana_transaction_decoder.rs, for example. Or move the model into a separate file and keep this method next to it.
Reasoning:
Utils is a tricky name. It's like a synonym for the "staff we don't know where to put, how to call and what it does". Having utils anywhere in the codebase usually doesn't seem right. I can't imagine any case when this will be a good naming, although we already have a number of utils throughout the codebase.
entities/src/utils.rs
Outdated
| .map(|uttb| TransactionTokenBalance { | ||
| account_index: uttb.account_index, | ||
| mint: uttb.mint, | ||
| ui_token_amount: uttb.ui_token_amount, | ||
| owner: Option::<String>::from(uttb.owner).unwrap_or_default(), | ||
| program_id: Option::<String>::from(uttb.program_id).unwrap_or_default(), | ||
| }) |
There was a problem hiding this comment.
Is it the same mapper as above? Maybe we should put it into a small method like txTokenBalanceFrom?
armyhaylenko
force-pushed
the
feature/MTG-1298-data-formats
branch
from
dfb6a50 to
88d66d8
Compare
nft_ingester/src/processors/transaction_based/bubblegum_updates_processor.rs
Show resolved
Hide resolved
No description provided.