-
Notifications
You must be signed in to change notification settings - Fork 325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[api] eth_getBlobSidecar #4371
[api] eth_getBlobSidecar #4371
Conversation
Quality Gate passedIssues Measures |
api/coreservice.go
Outdated
return res, nil | ||
} | ||
|
||
func (core *coreService) getBlobSidecars(height uint64) (blobs []any, txIndexes []uint64, txHashes []hash.Hash256) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this func should only return blobs
txIndexes, txHashes
should come from core's other API funcs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also txIndexes --> txIndices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
2d48f04
to
b907007
Compare
api/coreservice.go
Outdated
@@ -1213,6 +1217,58 @@ func (core *coreService) getBlockByHeight(height uint64) (*apitypes.BlockWithRec | |||
}, nil | |||
} | |||
|
|||
func (core *coreService) BlobSidecarsByHeight(height uint64) ([]*apitypes.BlobSidecarResult, error) { | |||
header, err := core.bc.BlockHeaderByHeight(height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to L1238
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
api/coreservice.go
Outdated
txIndices = append(txIndices, uint64(index)) | ||
} | ||
blkHash := header.HashBlock() | ||
for i, blob := range blobs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move L1232 to here, get the index
and use it at L1244, then we don't need to define txIndices
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
api/coreservice.go
Outdated
res = append(res, &apitypes.BlobSidecarResult{ | ||
BlobSidecar: blob, | ||
BlockNumber: height, | ||
BlockHash: common.BytesToHash(blkHash[:]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlockNumber, BlockHash
is common for all tx, only need 1 copy, i mean like below?
apitypes.BlobSidecarResult {
BlockNumber
BlockHash
[] struct {
blobSidecar
txIndex
txHash
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just keep same as bsc. IMO, this design is to use same structure for getSidercarByBlock and getSidercarByTx
case nil: | ||
return res, nil | ||
case ErrNotFound: | ||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ErrNotFound
should return nil to caller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, just like getBlock and getTransaction
BlockNumber: height, | ||
BlockHash: common.BytesToHash(blkHash[:]), | ||
TxIndex: uint64(index), | ||
TxHash: common.BytesToHash(txHashes[i][:]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the conversion in line 1230 and save them in local variables.
Quality Gate passedIssues Measures |
Description
introduce new api
eth_getBlobSidecars
to return blobs in a block.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: