-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
core: lookup txs by block number instead of block hash #19431
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ | |
package rawdb | ||
|
||
import ( | ||
"math/big" | ||
|
||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core/types" | ||
"github.com/ethereum/go-ethereum/ethdb" | ||
|
@@ -27,28 +29,36 @@ import ( | |
|
||
// ReadTxLookupEntry retrieves the positional metadata associated with a transaction | ||
// hash to allow retrieving the transaction or receipt by hash. | ||
func ReadTxLookupEntry(db ethdb.Reader, hash common.Hash) common.Hash { | ||
func ReadTxLookupEntry(db ethdb.Reader, hash common.Hash) *uint64 { | ||
data, _ := db.Get(txLookupKey(hash)) | ||
if len(data) == 0 { | ||
return common.Hash{} | ||
return nil | ||
} | ||
// | ||
karalabe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if len(data) < common.HashLength { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am thinking is this the final step of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing was updated to use this format: https://github.com/ethereum/go-ethereum/pull/19431/files#diff-0fba94aa54693ff2a4f26210c404dbd9R59 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not clear what you mean by a flag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean the "flag" for blob. For example, if the blob can be decoded into v3 structure, this is kind of "flag". Also if the length of blob is 32, it is kind of flag for v4, v5. So probably in the future if we want to change the format again, it can be painful. But just thinking. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, the flag here is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rjl493456442 My intention was how @karalabe interpreted the code. If either of you feel like this needs to be made more clear and have a suggestion about how to do it please feel free to adjust it or let me know what to do. |
||
var number big.Int | ||
number.SetBytes(data) | ||
numberU64 := number.Uint64() | ||
return &numberU64 | ||
karalabe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
// Database v4 tx lookup format just stores the hash. | ||
if len(data) == common.HashLength { | ||
return common.BytesToHash(data) | ||
return ReadHeaderNumber(db, common.BytesToHash(data)) | ||
} | ||
// Probably it's legacy txlookup entry data, try to decode it. | ||
// Finally try database v3 tx lookup format. | ||
var entry LegacyTxLookupEntry | ||
if err := rlp.DecodeBytes(data, &entry); err != nil { | ||
log.Error("Invalid transaction lookup entry RLP", "hash", hash, "blob", data, "err", err) | ||
return common.Hash{} | ||
return nil | ||
} | ||
return entry.BlockHash | ||
return &entry.BlockIndex | ||
} | ||
|
||
// WriteTxLookupEntries stores a positional metadata for every transaction from | ||
// a block, enabling hash based transaction and receipt lookups. | ||
func WriteTxLookupEntries(db ethdb.Writer, block *types.Block) { | ||
for _, tx := range block.Transactions() { | ||
if err := db.Put(txLookupKey(tx.Hash()), block.Hash().Bytes()); err != nil { | ||
if err := db.Put(txLookupKey(tx.Hash()), block.Number().Bytes()); err != nil { | ||
log.Crit("Failed to store transaction lookup entry", "err", err) | ||
} | ||
} | ||
|
@@ -62,12 +72,12 @@ func DeleteTxLookupEntry(db ethdb.Writer, hash common.Hash) { | |
// ReadTransaction retrieves a specific transaction from the database, along with | ||
// its added positional metadata. | ||
func ReadTransaction(db ethdb.Reader, hash common.Hash) (*types.Transaction, common.Hash, uint64, uint64) { | ||
blockHash := ReadTxLookupEntry(db, hash) | ||
if blockHash == (common.Hash{}) { | ||
blockNumber := ReadTxLookupEntry(db, hash) | ||
if blockNumber == nil { | ||
return nil, common.Hash{}, 0, 0 | ||
} | ||
blockNumber := ReadHeaderNumber(db, blockHash) | ||
if blockNumber == nil { | ||
blockHash := ReadCanonicalHash(db, *blockNumber) | ||
if blockHash == (common.Hash{}) { | ||
return nil, common.Hash{}, 0, 0 | ||
} | ||
body := ReadBody(db, blockHash, *blockNumber) | ||
|
@@ -88,12 +98,12 @@ func ReadTransaction(db ethdb.Reader, hash common.Hash) (*types.Transaction, com | |
// its added positional metadata. | ||
func ReadReceipt(db ethdb.Reader, hash common.Hash, config *params.ChainConfig) (*types.Receipt, common.Hash, uint64, uint64) { | ||
// Retrieve the context of the receipt based on the transaction hash | ||
blockHash := ReadTxLookupEntry(db, hash) | ||
if blockHash == (common.Hash{}) { | ||
blockNumber := ReadTxLookupEntry(db, hash) | ||
if blockNumber == nil { | ||
return nil, common.Hash{}, 0, 0 | ||
} | ||
blockNumber := ReadHeaderNumber(db, blockHash) | ||
if blockNumber == nil { | ||
blockHash := ReadCanonicalHash(db, *blockNumber) | ||
if blockHash == (common.Hash{}) { | ||
return nil, common.Hash{}, 0, 0 | ||
} | ||
// Read all the receipts from the block and return the one with the matching hash | ||
|
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.
Would it be worthwhile to have two methods? One that returns a
uint64
and one that returnshash
? That way, we would sometimes not needs to do lookup tx -> hash, read hashto number -> read cannon hash (number).It would also make the code for the callers a bit simpler
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.
@holiman Could you point me to the code that you think would benefit from this? How performance critical is that code as well?
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 was thinking about
ReadTransaction
andReadReceipt
.They do
ReadTxLookupEntry, ReadCanonicalHash,ReadBody
.If it's an old tx, it would internally be
read a hash, ReadHeaderNumber, ReadCanonicalHash,ReadBody
.which could be
read a hash -> ReadBody
.Maybe it would just overcomplicate things
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.
@holiman I agree that legacy databases pay a penalty here. If a user was using the client for constantly looking up transactions they may feel the effects of this slowdown. However, if this was a serious user I would probably expect them to resync. We could also offer a CLI tool to do these various database upgrades. @karalabe let me know if this is something that would be valuable to have. It would be done in a follow up PR.