Make fields not nullable #137
Conversation
📝 WalkthroughWalkthroughThis pull request introduces comprehensive changes across multiple Apibara packages, focusing on making various data structure properties required instead of optional. The modifications span three main packages: Beaconchain, EVM, and Starknet. The changes primarily involve updating type definitions in block-related files, enforcing stricter type requirements by converting optional properties to required ones. Additionally, new prerelease JSON configuration files were added for each package, indicating a coordinated update process. Changes
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
examples/starknet-client/src/main.ts (1)
57-62: Consider performance implications of broad filtering.The filter configuration now includes all possible data types with minimal filtering. While this provides comprehensive data access, it might impact performance due to increased data volume. Consider:
- Adding specific filters based on your actual data needs
- Implementing pagination or batch processing for large datasets
examples/beaconchain-client/src/main.ts (1)
41-42: Consider adding type validation for filter objects.The empty objects in the filter arrays (
validators: [{}], transactions: [{}]) will match all records without any filtering. While this might be intentional for the example, it could be helpful to:
- Add comments explaining that this configuration matches all records
- Show examples of actual filter criteria for better developer guidance
- validators: [{}], - transactions: [{}], - blobs: [{}], + // Match all validators, transactions, and blobs + // For specific filtering, use criteria like: + validators: [{ validatorIndex: 123 }], + transactions: [{ from: "0x..." }], + blobs: [{ blobIndex: 0 }],packages/beaconchain/src/block.ts (1)
74-91: Review mixed optionality in Transaction structure.The Transaction structure has an inconsistent mix of required and optional fields:
- Required: filterIds, transactionHash, nonce, from, value, input, accessList, transactionType
- Optional: to, gasPrice, gasLimit, maxFeePerGas, maxPriorityFeePerGas, signature, chainId, maxFeePerBlobGas
Please document the rationale for keeping certain fields optional while making others required.
Consider grouping optional fields based on their relationship to specific transaction types or scenarios to improve code maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
change/@apibara-beaconchain-c34099f9-dc52-429f-ab71-d12ed169b2cc.json(1 hunks)change/@apibara-evm-792c8a2f-e1df-4606-9ba7-a2dacb1264aa.json(1 hunks)change/@apibara-starknet-50e54de7-179b-4570-bbb1-ce0627ba8466.json(1 hunks)examples/beaconchain-client/src/main.ts(1 hunks)examples/evm-client/src/main.ts(2 hunks)examples/starknet-client/src/main.ts(3 hunks)packages/beaconchain/src/block.ts(1 hunks)packages/evm/src/block.ts(2 hunks)packages/starknet/src/block.ts(17 hunks)
✅ Files skipped from review due to trivial changes (2)
- change/@apibara-starknet-50e54de7-179b-4570-bbb1-ce0627ba8466.json
- change/@apibara-beaconchain-c34099f9-dc52-429f-ab71-d12ed169b2cc.json
🔇 Additional comments (25)
examples/starknet-client/src/main.ts (2)
67-67: Document finality change implications.Switching to "pending" finality means processing unconfirmed data, which could lead to:
- Potential data inconsistencies
- Need for reorg handling
- Different behavior in production vs testing
Consider adding a comment explaining the rationale for this change and any necessary error handling.
Also applies to: 69-69
3-3: Verify cleanup of unused imports.The imports have been simplified to match the current functionality. Let's verify no other unused imports remain in the codebase.
✅ Verification successful
Import cleanup verified successfully
The removed imports are properly cleaned up in
examples/starknet-client/src/main.ts. The only remaining references to these imports are in commented-out code, which is acceptable.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the removed imports rg "decodeEvent|getSelector|formatUnits" --type tsLength of output: 1600
packages/evm/src/block.ts (6)
87-89:⚠️ Potential issueEnsure
Signatureproperties are correctly providedAll properties in
Signature(r,s,v) are now required. Confirm that signature handling code provides these values to prevent deserialization errors.To check for any instances where
Signaturemight be missing required fields:✅ Verification successful
Signature handling is properly implemented
While the
Signatureschema inblock.tsnow requires all fields, the underlying proto implementation inproto/data.tsproperly handles partial objects and undefined values through:
- Null-safe initialization in
createBaseSignature()- Proper undefined checks in serialization methods
- Safe partial object handling in
fromPartial()🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of Signature where properties might be missing ast-grep --pattern 'Signature { r: $_, s: $_, v: $_, $$$ }'Length of output: 69
Script:
#!/bin/bash # Find Signature type definition and usages rg -l "Signature" --type ts # Find object literals that might be Signature instances ast-grep --pattern '{ r: $_, s: $_, v: $_ }' # Find potential Signature construction or assignments rg "Signature.*[{]" -A 3 --type tsLength of output: 5928
47-58:⚠️ Potential issueVerify that making fields required in
BlockHeaderdoes not introduce breaking changesChanging previously optional fields to required in
BlockHeadermay cause issues if any data sources or deserialization processes provide incomplete data. Ensure that all data providers supply these fields consistently to prevent runtime errors.To verify that all usages of
BlockHeaderin the codebase handle these required fields appropriately, please run the following script:✅ Verification successful
Making fields required in
BlockHeaderis safe and does not introduce breaking changesThe codebase implements a robust two-layer architecture where:
- Protocol buffer definitions maintain backward compatibility with optional fields
- Schema definitions enforce type safety with required fields
- Transformation methods handle the conversion safely with default values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for all deserialization of BlockHeader and check for handling of required fields # Find all files that import BlockHeader fd --type f ".*\.ts$" | xargs grep -l "BlockHeader" # Alternatively, check for any instances where BlockHeader is constructed without these fields ast-grep --pattern 'BlockHeader { blockHash: $_, parentBlockHash: $_, unclesHash: $_, miner: $_, stateRoot: $_, transactionsRoot: $_, receiptsRoot: $_, logsBloom: $_, difficulty: $_, gasLimit: $_, gasUsed: $_, timestamp: $_, $$$ }'Length of output: 508
Script:
#!/bin/bash # Check proto files for BlockHeader definitions and optional fields echo "=== Checking proto/data.ts files ===" for f in packages/*/src/proto/data.ts; do echo "=== $f ===" rg "BlockHeader" -A 10 -B 10 "$f" done echo -e "\n=== Checking block.ts files ===" for f in packages/*/src/block.ts; do echo "=== $f ===" rg "BlockHeader" -A 10 -B 10 "$f" done # Look specifically for optional fields echo -e "\n=== Checking for optional fields ===" rg "\?: " packages/*/src/proto/data.ts packages/*/src/block.tsLength of output: 59144
73-78:⚠️ Potential issueEnsure all
Withdrawalinstances provide required fieldsThe properties
filterIds,address, andamountinWithdrawalare now required. Verify that all parts of the application creating or processingWithdrawalinstances supply these fields to avoid runtime errors.Here is a script to help verify that all
Withdrawalinstances have these fields:
133-141:⚠️ Potential issueEnsure
Loginstances include new required fieldsFields
filterIds,address,transactionHash, andtransactionStatusinLogare now required. Verify that all logs include these fields to prevent issues during processing.Use the following script to check
Logusage:✅ Verification successful
Schema system enforces required Log fields
The Schema-based type system ensures that all Log instances must include the required fields with correct types. The
logsarray in the Block schema validates this structure at runtime.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all Log instances have required fields ast-grep --pattern 'Log { filterIds: $_, address: $_, topics: $_, data: $_, logIndex: $_, logIndexInTransaction: $_, transactionIndex: $_, transactionHash: $_, transactionStatus: $_, $$$ }'Length of output: 211
Script:
#!/bin/bash # Find Log definition and usage echo "=== Log Definition ===" ast-grep --pattern 'type Log = $_' ast-grep --pattern 'interface Log $_' ast-grep --pattern 'class Log $_' echo -e "\n=== Log Usage ===" rg "new Log\(" -A 5 -B 5 echo -e "\n=== Log References ===" rg "\blog\b.*:.*Log\b" -A 2 -B 2Length of output: 406
Script:
#!/bin/bash # Find Log.Type definition and usage patterns echo "=== Log Type Definition ===" rg "Log\.Type" -A 10 -B 10 echo -e "\n=== Object Literals with Log Fields ===" rg "filterIds.*address.*topics.*data.*logIndex" -A 10 echo -e "\n=== Schema Usage with Log ===" rg "Schema\.Object\(\{.*Log" -A 15 -B 2Length of output: 1455
147-147:⚠️ Potential issueVerify that
Blockobjects always includeheaderThe
headerproperty inBlockis now required. Ensure that all blocks include aheaderand that any code constructingBlockinstances provides theheaderfield.To find instances where
Blockmight be constructed without aheader:✅ Verification successful
Block schema correctly enforces required header field
The Block schema is defined using Effect Schema's
Schema.Structwhich enforces the presence of all required fields at runtime. The header field is correctly marked as required (non-optional) in the schema definition, ensuring that all Block instances must include a header.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for Block instances missing required header field ast-grep --pattern 'Block { header: $_, $$$ }'Length of output: 52
Script:
#!/bin/bash # Search for Block type definitions and instantiations echo "=== Block type/interface definitions ===" ast-grep --pattern 'interface Block { $$$ }' echo -e "\n=== Block class definitions ===" ast-grep --pattern 'class Block { $$$ }' echo -e "\n=== Block type assignments ===" ast-grep --pattern 'type Block = { $$$ }' echo -e "\n=== Block instantiations ===" rg "new Block\(" -A 5 echo -e "\n=== Block object literals ===" rg "as Block" -A 5Length of output: 4895
Script:
#!/bin/bash # Check the specific Block type in the EVM package echo "=== Block type in packages/evm/src/block.ts ===" cat packages/evm/src/block.ts echo -e "\n=== Usage of Block from block.ts ===" rg -l "from ['\"].*block['\"]" --type tsLength of output: 5149
94-112:⚠️ Potential issueReview required fields in
Transactionschema for potential impactsSeveral fields in
Transactionare now required, includingfilterIds,transactionHash,from,value,gas, andtransactionType. This may affect code that constructsTransactionobjects or processes them. Ensure that these fields are always available and supplied properly.To verify that all
Transactioninstances provide the required fields, run:✅ Verification successful
Required fields in
Transactionschema are properly handledThe review comment's concern has been investigated and no issues were found. The required fields in the
Transactionschema are properly handled through:
- Schema validation via the Schema.Struct definition
- Proper initialization in protobuf implementations
- Default values provided where appropriate
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all instances of Transaction creation and ensure required fields are provided ast-grep --pattern 'Transaction { filterIds: $_, transactionIndex: $_, transactionHash: $_, nonce: $_, from: $_, to: $_, value: $_, gasPrice: $_, gas: $_, maxFeePerGas: $_, maxPriorityFeePerGas: $_, input: $_, signature: $_, chainId: $_, accessList: $_, transactionType: $_, $$$ }'Length of output: 317
Script:
#!/bin/bash # Find Transaction-related files and their context rg -l "Transaction" --type ts # Look for Transaction object creation or usage patterns ast-grep --pattern 'new Transaction($_)' # Look for object creation patterns that might match Transaction ast-grep --pattern '{ filterIds: $_, from: $_, gas: $_, $$$ }' # Search for schema validation code rg "Schema\." -A 5 --type tsLength of output: 75282
Script:
#!/bin/bash # Find Transaction construction and usage patterns rg -A 5 "new Transaction\(" --type ts rg -A 5 "Transaction\.make" --type ts rg -A 5 "Transaction\(" --type ts # Look for test files that might show Transaction usage rg -l "Transaction" --type test # Search for validation or error handling rg -A 5 "validateTransaction|validateFields|validate.*Transaction" --type tsLength of output: 21441
packages/starknet/src/block.ts (12)
84-85:⚠️ Potential issueVerify
ResourceBoundsMappingincludes requiredl1Gasandl2GasThe fields
l1Gasandl2GasinResourceBoundsMappingare now required. Ensure these are provided in all instances whereResourceBoundsMappingis used.Use the following script to verify:
✅ Verification successful
Required fields
l1Gasandl2Gasare properly enforcedThe
ResourceBoundsMappingSchema.Struct in block.ts correctly enforces bothl1Gasandl2Gasas required fields. The implementation is consistent across all usages in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check that ResourceBoundsMapping includes l1Gas and l2Gas ast-grep --pattern 'ResourceBoundsMapping { l1Gas: $_, l2Gas: $_, $$$ }'Length of output: 80
Script:
#!/bin/bash # Find ResourceBoundsMapping definition and usages echo "=== Finding ResourceBoundsMapping definition ===" rg "type\s+ResourceBoundsMapping" -A 5 echo -e "\n=== Finding ResourceBoundsMapping usages ===" rg "ResourceBoundsMapping" -B 2 -A 2 echo -e "\n=== Finding interface/type patterns ===" ast-grep --pattern 'type ResourceBoundsMapping = { $$$ }' ast-grep --pattern 'interface ResourceBoundsMapping { $$$ }'Length of output: 14507
467-475:⚠️ Potential issueConfirm required fields in
MessageToL1are providedThe
MessageToL1schema now requires fieldsfilterIds,fromAddress,toAddress,payload,messageIndex,transactionIndex,transactionHash,transactionStatus, andmessageIndexInTransaction. Ensure these fields are included in allMessageToL1instances.Check with:
✅ Verification successful
All required fields in
MessageToL1are properly definedThe schema correctly defines all required fields without using optional markers. The implementation is consistent across the codebase, including the protobuf definitions and TypeScript interfaces.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify MessageToL1 instances include required fields ast-grep --pattern 'MessageToL1 { filterIds: $_, fromAddress: $_, toAddress: $_, payload: $_, messageIndex: $_, transactionIndex: $_, transactionHash: $_, transactionStatus: $_, messageIndexInTransaction: $_, $$$ }'Length of output: 237
Script:
#!/bin/bash # First find all relevant files echo "=== Files containing MessageToL1 ===" rg "MessageToL1" --type ts echo -e "\n=== Type definitions ===" # Search for type/interface definitions ast-grep --pattern 'type MessageToL1 = { $$$ }' ast-grep --pattern 'interface MessageToL1 { $$$ }' echo -e "\n=== Object creations ===" # Search for object creations with MessageToL1 type rg "MessageToL1" -A 10 -B 2Length of output: 41547
373-377:⚠️ Potential issueEnsure
TransactionReceiptMetarequired fields are providedFields
transactionIndex,transactionHash,actualFee,executionResources, andexecutionResultare now required inTransactionReceiptMeta. Verify that all receipt instances include these fields.Check usage with:
✅ Verification successful
Required fields in
TransactionReceiptMetaare properly enforcedThe fields are correctly required through:
- Schema.Struct definition in block.ts
- Required fields in the Protocol Buffer message
- Proper encoding/decoding implementation
The apparent optionality in the TypeScript interface is due to protobuf-ts generation patterns and doesn't affect the runtime requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify TransactionReceiptMeta includes required fields ast-grep --pattern 'TransactionReceiptMeta { transactionIndex: $_, transactionHash: $_, actualFee: $_, executionResources: $_, executionResult: $_, $$$ }'Length of output: 168
Script:
#!/bin/bash # Find TransactionReceiptMeta definition and usages # Find the type/interface definition echo "=== TransactionReceiptMeta Definition ===" ast-grep --pattern 'type TransactionReceiptMeta = $_' ast-grep --pattern 'interface TransactionReceiptMeta $_' # Find usages of TransactionReceiptMeta echo -e "\n=== TransactionReceiptMeta Usages ===" rg "TransactionReceiptMeta" -A 5 -B 5 # Look for object creation or assignments echo -e "\n=== Object Creation/Assignment Patterns ===" rg "as TransactionReceiptMeta|: TransactionReceiptMeta" -A 10Length of output: 22248
442-450:⚠️ Potential issueEnsure
Eventobjects include new required fieldsFields
filterIds,address,keys,data,eventIndex,transactionIndex,transactionHash,transactionStatus, andeventIndexInTransactioninEventare now required. Confirm that all events include these fields to avoid issues during event processing.Use the following script to check
Eventusage:✅ Verification successful
Event fields are correctly identified as required
The review comment is accurate. The Event schema in
packages/starknet/src/block.tsenforces all fields (filterIds,address,keys,data,eventIndex,transactionIndex,transactionHash,transactionStatus, andeventIndexInTransaction) as required through runtime validation. While the TypeScript interface inproto/data.tsmarks these fields as optional for type-checking purposes, the schema validation ensures they are present at runtime.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all Event instances have required fields ast-grep --pattern 'Event { filterIds: $_, address: $_, keys: $_, data: $_, eventIndex: $_, transactionIndex: $_, transactionHash: $_, transactionStatus: $_, eventIndexInTransaction: $_, $$$ }'Length of output: 215
Script:
#!/bin/bash # Find Event type/interface definition and usage echo "=== Event Definition ===" ast-grep --pattern 'interface Event { $$$ }' echo -e "\n=== Event Type Definition ===" ast-grep --pattern 'type Event = { $$$ }' echo -e "\n=== Event Usage ===" rg "Event" -A 5 -B 5 echo -e "\n=== Event Object Creation ===" ast-grep --pattern 'const $_ : Event = { $$$ }'Length of output: 66636
586-586:⚠️ Potential issueVerify that
Blockobjects include the requiredheaderfieldThe
headerproperty inBlockis now required. Ensure that all block instances provide theheaderfield to avoid issues during block processing.To find instances where
Blockmight be missing theheader:✅ Verification successful
Block header field is correctly marked as required
The implementation in
packages/starknet/src/block.tsconfirms that theheaderfield is required in the Block schema. This matches the review comment's assertion and ensures type safety for block processing.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for Block instances missing required header field ast-grep --pattern 'Block { header: $_, $$$ }'Length of output: 52
Script:
#!/bin/bash # Find Block type/interface definition and usages echo "=== Block type definition ===" ast-grep --pattern 'interface Block { $$$ }' echo -e "\n=== Block type usages ===" rg "new Block" -A 2 echo -e "\n=== Block header references ===" rg "Block.*header" -A 2Length of output: 9952
Script:
#!/bin/bash # Check Block implementation in block.ts echo "=== Block class definition ===" cat packages/starknet/src/block.ts echo -e "\n=== Other Block class definitions ===" ast-grep --pattern 'class Block { $$$ }'Length of output: 18035
498-500:⚠️ Potential issueEnsure
StorageDiffinstances provide required fieldsFields
filterIds,contractAddress, andstorageEntriesinStorageDiffare now required. Verify that all storage diffs include these fields.Use the following script:
✅ Verification successful
Schema validation ensures required fields are present
The Schema definition in
block.tscorrectly enforces that all StorageDiff instances must includefilterIds,contractAddress, andstorageEntriesfields. The Schema.Struct definition does not use Schema.optional, making these fields required. While the protobuf-generated interface shows these as optional, the Schema validation layer ensures compliance with the requirements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check StorageDiff instances for required fields ast-grep --pattern 'StorageDiff { filterIds: $_, contractAddress: $_, storageEntries: $_, $$$ }'Length of output: 106
Script:
#!/bin/bash # Search for StorageDiff type definitions and usages echo "=== StorageDiff type definitions ===" ast-grep --pattern 'type StorageDiff = $_' echo -e "\n=== StorageDiff interface/class definitions ===" ast-grep --pattern 'interface StorageDiff { $$$ }' echo -e "\n=== StorageDiff object creations ===" rg "StorageDiff" -A 5 -B 5 echo -e "\n=== Files containing StorageDiff ===" fd . -t f -x grep -l "StorageDiff" {} \;Length of output: 30662
556-557:⚠️ Potential issueVerify
ContractChangeincludes requiredfilterIdsandchangeFields
filterIdsandchangeinContractChangeare now required. Ensure that all contract changes include these fields to maintain data integrity.Check usage with:
✅ Verification successful
Schema definition confirms required fields
The Schema definition in
packages/starknet/src/block.tsenforces bothfilterIdsandchangeas required fields inContractChange, validating the review comment's concern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify ContractChange instances have required fields ast-grep --pattern 'ContractChange { filterIds: $_, change: $_, $$$ }'Length of output: 78
Script:
#!/bin/bash # Search for ContractChange definition and usages echo "=== ContractChange Definition ===" rg "ContractChange" -A 5 -B 5 echo -e "\n=== ContractChange Type/Interface Definition ===" ast-grep --pattern 'interface ContractChange { $$$ }' ast-grep --pattern 'type ContractChange = { $$$ }' echo -e "\n=== ContractChange Schema Definition ===" ast-grep --pattern 'const ContractChange = Schema.$$$' echo -e "\n=== ContractChange Usages ===" rg "ContractChange\(" -A 2Length of output: 30676
292-305:⚠️ Potential issueVerify that
Transactioninstances correctly provide required fieldsThe
Transactionschema now requiresfilterIds,meta, and specifies required fields in transaction variants. Ensure that all transaction instances comply with the updated schema and that any code constructing transactions includes the required fields.To verify usage:
✅ Verification successful
Transaction schema requirements are properly enforced
The implementation ensures that all Transaction instances comply with the schema requirements through:
- Schema.Struct validation enforcing the required fields
- Proper initialization through proto methods that handle all required fields
- No evidence of direct object creation bypassing the schema validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check that all Transaction instances conform to the new required fields ast-grep --pattern 'Transaction { filterIds: $_, meta: $_, transaction: $_, $$$ }'Length of output: 92
Script:
#!/bin/bash # Find Transaction type usage and creation patterns echo "=== Transaction type usage ===" rg "Transaction[^a-zA-Z]" --type ts -A 2 echo -e "\n=== Transaction object creation ===" rg "new Transaction|as Transaction|: Transaction =" --type ts -A 2 echo -e "\n=== Transaction imports ===" rg "import.*Transaction" --type tsLength of output: 56571
Script:
#!/bin/bash # Find transaction construction and usage in tests and implementation echo "=== Transaction construction in tests ===" rg "filterIds.*meta.*transaction" --type ts -A 5 echo -e "\n=== Transaction usage in implementation ===" rg "createBaseTransaction|Transaction\.fromPartial|Transaction\.create" --type ts -A 2Length of output: 11113
123-131:⚠️ Potential issueConfirm all
BlockHeaderrequired fields are providedFields like
parentBlockHash,sequencerAddress,timestamp,starknetVersion,l1GasPrice,l1DataGasPrice, andl1DataAvailabilityModeare now required inBlockHeader. Verify that these fields are always present inBlockHeaderinstances.Check with:
✅ Verification successful
Required fields are correctly enforced in BlockHeader schema
The BlockHeader schema in
packages/starknet/src/block.tsproperly enforces all the mentioned fields as required through the Schema.Struct definition. OnlynewRootis explicitly marked as optional, whileparentBlockHash,sequencerAddress,timestamp,starknetVersion,l1GasPrice,l1DataGasPrice, andl1DataAvailabilityModeare all required fields.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure BlockHeader instances include all required fields ast-grep --pattern 'BlockHeader { blockHash: $_, parentBlockHash: $_, blockNumber: $_, sequencerAddress: $_, newRoot: $_, timestamp: $_, starknetVersion: $_, l1GasPrice: $_, l1DataGasPrice: $_, l1DataAvailabilityMode: $_, $$$ }'Length of output: 252
Script:
#!/bin/bash # Find BlockHeader definition ast-grep --pattern 'interface BlockHeader { $$$ }' echo "=== Searching for class definition ===" ast-grep --pattern 'class BlockHeader { $$$ }' echo "=== Searching for type definition ===" ast-grep --pattern 'type BlockHeader = { $$$ }' echo "=== Looking for BlockHeader instantiations ===" rg "new BlockHeader" -A 5 echo "=== Looking for potential factory methods ===" rg "BlockHeader" -B 2 -A 5Length of output: 42876
568-570:⚠️ Potential issueEnsure
NonceUpdateinstances provide required fieldsFields
filterIds,contractAddress, andnonceinNonceUpdateare now required. Confirm that all nonce updates include these fields to prevent inconsistencies.Use the following script to verify:
✅ Verification successful
Schema enforces required fields for NonceUpdate
The
NonceUpdateschema inblock.tsexplicitly requires all fields (filterIds,contractAddress, andnonce) throughSchema.Structwithout any optional fields. The implementation ensures that all nonce updates must include these fields to pass schema validation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check NonceUpdate instances for required fields ast-grep --pattern 'NonceUpdate { filterIds: $_, contractAddress: $_, nonce: $_, $$$ }'Length of output: 97
Script:
#!/bin/bash # Search for NonceUpdate in various contexts echo "=== Searching for NonceUpdate type/class definitions ===" rg "NonceUpdate" -A 5 -B 5 echo -e "\n=== Searching for NonceUpdate instantiation patterns ===" ast-grep --pattern "new NonceUpdate" echo -e "\n=== Searching for NonceUpdate as a type annotation ===" ast-grep --pattern ": NonceUpdate" echo -e "\n=== Looking for files containing NonceUpdate ===" fd -t f | xargs rg "NonceUpdate" -lLength of output: 27934
79-80:⚠️ Potential issueEnsure required fields in
ResourceBoundsare always providedFields
maxAmountandmaxPricePerUnitinResourceBoundsare now required. Confirm that all instances ofResourceBoundsprovide these fields to avoid runtime errors.To check the usage of
ResourceBounds:✅ Verification successful
Schema correctly enforces required fields while maintaining compatibility
The implementation is well-designed:
- Schema validation ensures required fields are present at runtime
- Protocol layer maintains backward compatibility with optional fields
- Encoding/decoding logic properly handles all cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that ResourceBounds instances have required fields ast-grep --pattern 'ResourceBounds { maxAmount: $_, maxPricePerUnit: $_, $$$ }'Length of output: 87
Script:
#!/bin/bash # First, find all relevant files echo "=== Files containing ResourceBounds ===" rg -l "ResourceBounds" echo -e "\n=== ResourceBounds type usage ===" rg "ResourceBounds" -A 5 -B 5 echo -e "\n=== Object creation patterns ===" # Try different object creation patterns ast-grep --pattern 'resourceBounds: { $$$ }' ast-grep --pattern 'resourceBounds = { $$$ }' # Look for type annotations ast-grep --pattern ': ResourceBounds = { $$$ }'Length of output: 35737
Script:
#!/bin/bash # Look for test files and actual usage echo "=== Test files containing ResourceBounds ===" rg -l "ResourceBounds" --glob "**/*test*.*" echo -e "\n=== ResourceBounds instantiation in tests ===" rg "ResourceBounds" -A 10 -B 10 --glob "**/*test*.*" echo -e "\n=== Direct object creation patterns ===" rg "resourceBounds.*=.*{" -A 5 rg "ResourceBounds.*=.*{" -A 5Length of output: 671
Script:
#!/bin/bash # Search for object literals and field usage echo "=== Object literals with required fields ===" rg "maxAmount.*maxPricePerUnit|maxPricePerUnit.*maxAmount" -A 5 -B 5 echo -e "\n=== Resource bounds assignments ===" rg "resourceBounds" -A 5 -B 5 echo -e "\n=== Function calls with resource bounds ===" ast-grep --pattern 'resourceBounds: $_'Length of output: 23298
145-147:⚠️ Potential issueEnsure
TransactionMetaincludes required fieldsFields
transactionIndex,transactionHash, andtransactionStatusare now required inTransactionMeta. Confirm these are provided in all transactions.Check with:
✅ Verification successful
TransactionMeta fields are properly enforced by Schema validation
The Schema.Struct definition in block.ts ensures that
transactionIndex,transactionHash, andtransactionStatusare required fields. While the proto interface marks these fields as optional for wire format compatibility, the Schema validation layer enforces their presence when the data is processed through the application's type system.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify TransactionMeta includes required fields ast-grep --pattern 'TransactionMeta { transactionIndex: $_, transactionHash: $_, transactionStatus: $_, $$$ }'Length of output: 120
Script:
#!/bin/bash # Find TransactionMeta definition echo "=== Finding TransactionMeta definition ===" rg "type\s+TransactionMeta|interface\s+TransactionMeta" -A 10 echo -e "\n=== Finding TransactionMeta usage ===" rg "TransactionMeta" --type ts -C 2Length of output: 8010
Script:
#!/bin/bash # Look for transformation or validation logic between proto and schema echo "=== Searching for proto to schema transformations ===" rg -B 3 -A 3 "fromProto|toProto|validate" packages/starknet/src/ echo -e "\n=== Searching for Transaction usage patterns ===" rg "new\s+Transaction|Transaction\(" --type ts -C 2Length of output: 18197
Script:
#!/bin/bash # Search for Schema validation and error handling echo "=== Searching for Schema validation and error handling ===" rg "Schema\.(validate|parse|check)" packages/starknet/src/ -B 2 -A 2 echo -e "\n=== Looking for TransactionMeta usage in tests ===" rg "TransactionMeta" packages/starknet/src/**/*test*Length of output: 441
Script:
#!/bin/bash # Find test files and Schema usage echo "=== Finding test files ===" fd "test" packages/starknet/src/ echo -e "\n=== Looking for Schema imports and usage ===" rg "import.*Schema" packages/starknet/src/ -B 1 -A 3 echo -e "\n=== Looking for TransactionMeta in tests (adjusted path) ===" fd "test" packages/starknet/src/ -x rg "TransactionMeta" {}Length of output: 1821
Script:
#!/bin/bash # Look for Schema.Struct usage patterns echo "=== Finding Schema.Struct usage patterns ===" rg "Schema\.Struct" packages/starknet/src/ -B 2 -A 2 echo -e "\n=== Looking for validation or parsing code ===" rg "parse|decode|fromJSON|fromPartial" packages/starknet/src/block.ts -C 2Length of output: 23443
examples/evm-client/src/main.ts (2)
23-23: Update to the default EVM stream URLThe default stream URL has been changed to
"https://ethereum-sepolia.preview.apibara.org". Ensure this endpoint is valid, accessible, and updated throughout the documentation or related code if necessary.If the old URL is deprecated, consider updating any references to it elsewhere.
39-41: Update filter configuration for transactionsThe
filternow includes detailed transaction options to include receipts, logs, and all transaction statuses:transactions: [ { includeReceipt: true, transactionStatus: "all", includeLogs: true }, ],Ensure that this configuration aligns with the expected data from the stream and that the application handles the additional data appropriately.
packages/beaconchain/src/block.ts (3)
64-69: Consider the implications of optional signature fields.While other structures are being made more strict, the new
Signaturestructure introduces optional fields. This seems inconsistent with the PR's objective. Is there a specific reason these fields need to be optional?Could we make some of these fields required based on the transaction type or chain requirements?
Line range hint
97-101: LGTM: Block structure changes.The changes to make
headerrequired and keep arrays for validators, blobs, and transactions are appropriate. Empty arrays can represent the absence of these elements while maintaining type safety.
9-16: Verify backward compatibility for ExecutionPayload.Making these fields required is a breaking change. While it improves type safety, we need to ensure that all data sources (especially historical data) always provide these fields.
| // const { args, transactionHash } = decodeEvent({ | ||
| // abi, | ||
| // eventName: "Transfer", | ||
| // event, | ||
| // }); | ||
| // consola.info( | ||
| // `${prettyAddress(args.from)} -> ${prettyAddress(args.to)} ${formatUnits(args.value, 6)} ${colors.gray(transactionHash ?? "")}`, | ||
| // ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented code.
Instead of keeping commented code in the codebase:
- Remove it if it's no longer needed
- If it's for reference, move it to documentation
- If it's temporary, create a TODO issue to track its restoration
| filterIds: Schema.Array(Schema.Number), | ||
| transactionIndex: Schema.Number, | ||
| transactionHash: B256, | ||
| cumulativeGasUsed: U128, | ||
| gasUsed: U128, | ||
| effectiveGasPrice: U128, | ||
| from: Address, | ||
| to: Schema.optional(Address), | ||
| contractAddress: Schema.optional(Address), | ||
| logsBloom: Schema.optional(Bloom), | ||
| transactionType: Schema.optional(Schema.BigIntFromSelf), | ||
| logsBloom: Bloom, | ||
| transactionType: Schema.BigIntFromSelf, | ||
| blobGasUsed: Schema.optional(U128), | ||
| blobGasPrice: Schema.optional(U128), | ||
| transactionStatus: Schema.optional(TransactionStatus), | ||
| transactionStatus: TransactionStatus, |
There was a problem hiding this comment.
💡 Codebase verification
Schema and Implementation Mismatch for Required Fields in TransactionReceipt
The verification reveals a critical mismatch between the schema requirements and the implementation:
- The schema in
block.tscorrectly defines required fields (filterIds, transactionIndex, transactionHash, etc.) - However, the implementation in
proto/data.tsdoesn't enforce these requirements:createBaseTransactionReceiptinitializes required fields asundefinedfromPartialmethod doesn't validate the presence of required fields
This needs to be fixed to prevent potential runtime validation errors.
🔗 Analysis chain
Confirm that TransactionReceipt required fields are handled
With filterIds, transactionIndex, transactionHash, cumulativeGasUsed, gasUsed, effectiveGasPrice, and from now required in TransactionReceipt, ensure that all instances where TransactionReceipt is used provide these fields.
Check for proper handling by running:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all TransactionReceipt instances provide required fields
ast-grep --pattern 'TransactionReceipt {
filterIds: $_,
transactionIndex: $_,
transactionHash: $_,
cumulativeGasUsed: $_,
gasUsed: $_,
effectiveGasPrice: $_,
from: $_,
to: $_,
contractAddress: $_,
logsBloom: $_,
transactionType: $_,
transactionStatus: $_,
$$$
}'
Length of output: 290
Script:
#!/bin/bash
# Find TransactionReceipt imports and usages
echo "=== TransactionReceipt imports ==="
rg "import.*TransactionReceipt" -l
echo -e "\n=== TransactionReceipt type annotations ==="
rg ":\s*TransactionReceipt\b" -A 2
echo -e "\n=== TransactionReceipt instantiations ==="
# Less strict pattern matching for object creation
ast-grep --pattern 'new TransactionReceipt($_)'
# Alternative object creation pattern
ast-grep --pattern 'TransactionReceipt({$$$})'
echo -e "\n=== Spread operations with TransactionReceipt ==="
rg "\.\.\." -A 3 -B 3 $(rg "TransactionReceipt" -l)
Length of output: 4756
Script:
#!/bin/bash
# Check implementation details
echo "=== TransactionReceipt base implementation ==="
rg -A 20 "function createBaseTransactionReceipt\(\)" packages/evm/src/proto/data.ts
echo -e "\n=== Schema validation ==="
rg -A 10 "TransactionReceipt.*Schema\." packages/evm/src/
echo -e "\n=== fromPartial implementation ==="
rg -A 20 "fromPartial.*TransactionReceipt" packages/evm/src/proto/data.ts
Length of output: 3030
| "type": "prerelease", | ||
| "comment": "evm: make more fields required", | ||
| "packageName": "@apibara/evm", | ||
| "email": "[email protected]", | ||
| "dependentChangeType": "patch" |
There was a problem hiding this comment.
Consider using a major version bump instead of patch.
The changes to make fields required are breaking changes that affect backward compatibility. According to semver:
- A patch should only contain backward compatible bug fixes
- Breaking changes should trigger a major version bump
- "type": "prerelease",
- "dependentChangeType": "patch"
+ "type": "prerelease",
+ "dependentChangeType": "major"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "type": "prerelease", | |
| "comment": "evm: make more fields required", | |
| "packageName": "@apibara/evm", | |
| "email": "[email protected]", | |
| "dependentChangeType": "patch" | |
| "type": "prerelease", | |
| "comment": "evm: make more fields required", | |
| "packageName": "@apibara/evm", | |
| "email": "[email protected]", | |
| "dependentChangeType": "major" |
Summary
This PR makes all non-nullable fields non-nullable.
Previously, the packages followed the Protobuf wisdom of marking all fields as nullable to enable backward and forward compatibility of the protocol.
In practice, this degraded the user experience since they had to perform null checks themselves.