Skip to content
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

[wip] Why we even need txNum for statesyncTx? Yep. In RPC we needn't it. #14086

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

JkLondon
Copy link
Member

@JkLondon JkLondon commented Mar 5, 2025

No description provided.

@Giulio2002
Copy link
Contributor

Giulio2002 commented Mar 5, 2025

@AskAlexSharov i am scared of this PR

@AskAlexSharov
Copy link
Collaborator

me too. but i don't understand it yet. will dig today.

@@ -512,23 +512,19 @@ func (api *PrivateDebugAPIImpl) GetRawTransaction(ctx context.Context, txnHash c
txNumsReader := rawdbv3.TxNums.WithCustomReadTxNumFunc(freezeblocks.ReadTxNumFuncFromBlockReader(ctx, api._blockReader))

// Private API returns 0 if transaction is not found.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't understand why need rely on blockNum == 0 if txnLookup returns ok

@AskAlexSharov
Copy link
Collaborator

AskAlexSharov commented Mar 6, 2025

faced some issues on release3.0 branch:

  1. External rpcd can't start - because we didn't cherry pick set ready on file open as well as download complete #13901 . doing: cherry-pick: external rpcdaemon can't start #14090
  2. On amoy: curl 'http://localhost:8545' -H 'Content-Type: application/json' --data '{"id": 1, "jsonrpc": "2.0", "method": "eth_getLogs", "params": [ {"fromBlock": "14300401","toBlock": "14300501"} ]}' did panic EROR[03-06|03:25:55.613] RPC method eth_getLogs crashed: runtime error: index out of range [0] with length 0 [service.go:223 panic.go:785 panic.go:115 bor_receipts_generator.go:86 eth_receipts.go:306 eth_receipts.go:143 value.go:581 value.go:365 service.go:228 handler.go:534 handler.go:484 handler.go:425 handler.go:245 handler.go:338 asm_amd64.s:1700] - fixed by: nil ptr in gen bor logs #14091 and cherry-pick: nil ptr in gen bor logs  #14092
  3. CI get red recently: https://github.com/erigontech/erigon/commits/release/3.0

now can try to test this PR

@AskAlexSharov
Copy link
Collaborator

AskAlexSharov commented Mar 6, 2025

@Giulio2002
Copy link
Contributor

ok guys, please let's not merge this yet

@AskAlexSharov AskAlexSharov changed the title Why we even need txNum for statesyncTx? Yep. In RPC we needn't it. [wip] Why we even need txNum for statesyncTx? Yep. In RPC we needn't it. Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants