Block RPC PR Followup

[MitchTurner]

Linked Issues/PRs

Complete feedback from:
#3100
#3101
#3112
#3116

As they have all been merged into
#3100

TODOs

Completed

• Move proto types to new repo (or at least remove dep on protoc) Add Protobuf definitions fuel-core-protobuf#1
• Use ipv4 instead of ipv4 let listener = TcpListener::bind("[::1]:0").unwrap();
• make sure rpc is optional (may already be good with feature)
• "If we add a new database, we should include it in all places where we already interact with databases, like check_version, rollback_to, and so on(check where do we use databa liek relayer)"
• "The CombinedDatabase::check_version() method is missing a version check for the new block_aggregation database. This omission is inconsistent with other database checks and could lead to version compatibility issues."
• "I would like to mention the reason why this database doesn't force a monotonic increase in height."
• "Also, becuase we don't force monotonic height increase, we can't actually re-use rollback feature from historicat database. But we still need the ability to reset the state of the block aggregator to height X. So you can add another function, which we can call in rollback_to."
• "I know that ServiceRunner here is an overkill, but we have GraphQL that looks the same, and it was fiiiiine=D Maybe for consistency plus some logging we could reuse it here as well. It's just an internal service for your main service"
• // TODO: Should this be owned to begin with? Yeah, we should work with reference in proto_header_from_header and in proto_tx_from_tx
• maybe "If you used BoxStream, then you could avoid usage of the tokio::spawn( below. You can just do inner.map({...}).into_boxed()" in protobuf_adapater impl of protobuf trait: type GetBlockRangeStream = BoxStream<Result<ProtoBlockResponse, Status>>;
• #[allow(unused)] fn arb_inputs()
• in serializer_adapter.rs: "We should work with reference to everywhere in this file. And we should also avoid usage of unswap_or_default. All fields are always set, so it is strange why it can be None`
• if let Some(value) = policies.get(PolicyType::Owner) { values[5] = value; }
• in serializer_adapter.rs: "Why values for policies in an empty array? If the policy is not set, we shouldn't include them. If no policy is set, then it will be empty vector."
• in crates/types/src/blockchain/header.rs: "We don't need to expose the application header. You can find an example on how to create a header here: https://github.com/FuelLabs/fuel-rust-indexer/blob/main/crates/receipts_manager/adapters/graphql_event_adapter.rs#L250", "And the same for the block, you don't need manually calculate it, you can do that from the Block::new."
• in types/src/test_helpers.rs: "Block::new should be enough for you to create a header and a block. You don't need to implement all the logic by yourself.You can clean up getters and setters which you've added, after you udpated code here=)
  "
• Maybe remove self.go_to_sleep_before_continuing().await;

Need Feedback

• in serializer_adapter.rs: "If we use Rust definition to define variants inside of enums, then we can remove useless fields like data for MessageCoinPredicate and witness_index for MessageCoinPredicate" (@xgreenx I don't understand this. What "Rust definition"?)

Noop

• in serializer_adapter.rs: "I think instead of creating many procedure fucntions, we could implement TryFrom and From for types, plus, we could split them into its own folders by transactions and some common folder." (Since this is in a separate repo now, the TryFrom/From impls wouldn't be valid due to the orphan rule)
• in api.proto: "I think all txs have these, maybe we could move them to the top-level Transaction?" DRY up chargeable txs?( I don't really care for this abstraction, but I could be convinced to DRY this up. Just doesn't seem like much gain)
• I think we can move fetching of the full blocks to be a part of default functionality provided by FuelClient along with old functionality (in test-helpers/src/client_ext.rs) (I don't want to add new functionality in this work, we could do a followup)
• "[nit] https://github.com/FuelLabs/fuel-rust-indexer/blob/main/crates/receipts_manager/service.rs#L584 We can replace the whole service with stream of joined events(old + new)" (WE could do this as a followup)

Description

Checklist

• Breaking changes are clearly marked as such in the PR description and changelog
• New behavior is reflected in tests
• The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

• I have reviewed the code myself
• I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

• Rust SDK
• Sway compiler
• Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
• Someone else?

[MitchTurner]

in serializer_adapter.rs: "If we use Rust definition to define variants inside of enums, then we can remove useless fields like data for MessageCoinPredicate and witness_index for MessageCoinPredicate"

I don't get this.