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.

Sign up for GitHub

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

Jump to bottom

eth: make debug_StorageRangeAt take a block hash or number #27328

Merged
merged 1 commit into from
May 30, 2023
Merged

eth: make debug_StorageRangeAt take a block hash or number #27328

merged 1 commit into from
May 30, 2023

Conversation

jwasinger
Copy link
Contributor

No description provided.

holiman
holiman reviewed May 23, 2023
View reviewed changes
eth/api.go Outdated Show resolved Hide resolved
s1na
s1na reviewed May 25, 2023
View reviewed changes
eth/api.go Outdated
block := api.eth.blockchain.GetBlockByHash(blockHash)
func (api *DebugAPI) StorageRangeAt(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, txIndex int, contractAddress common.Address, keyStart hexutil.Bytes, maxResult int) (StorageRangeResult, error) {
var block *types.Block
if number, ok := blockNrOrHash.Number(); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

Or use api.eth.APIBackend.BlockByNumberOrHash. I'm not sure why we don't use backend so much in this file and directly use blockchain 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that looks like a good alternative

Copy link
Contributor Author

@jwasinger jwasinger May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I moved APIBackend.BlockByNumberOrHash/BlockByNumber methods into the Ethereum object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. that was uneccesary. Ethereum has a pointer to the APIBackend...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. should be good now.

@jwasinger jwasinger force-pushed the storage-range-hash-or-number branch from 6fc46c4 to 0c367b5 Compare May 26, 2023 12:42
s1na
s1na reviewed May 26, 2023
View reviewed changes
eth/api.go Outdated Show resolved Hide resolved
s1na
s1na approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM!

holiman
holiman approved these changes May 29, 2023
View reviewed changes
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman
Copy link
Contributor

holiman commented May 29, 2023

Sorry, I bitrotted this PR a bit, needs to apply the change against the file eth/api_debug.go now instead

@holiman holiman added this to the 1.12.1 milestone May 29, 2023
rjl493456442
rjl493456442 approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Although I think it's a breaking change, originally users send hash 0x1234 now {"blockHash": "0x1234"} is required. But it's a debug API anyway, it's totally fine to change it.

@jwasinger jwasinger force-pushed the storage-range-hash-or-number branch from 36b4b4b to d19a1a7 Compare May 30, 2023 09:57
@jwasinger @holiman @s1na
eth: make StorageRangeAt take a block hash or number
97d00ba 
Co-authored-by: Martin Holst Swende <[email protected]>
Co-authored-by: Sina Mahmoodi <[email protected]>
@jwasinger jwasinger force-pushed the storage-range-hash-or-number branch from d19a1a7 to 97d00ba Compare May 30, 2023 10:00
@s1na
Copy link
Contributor

s1na commented May 30, 2023

Although I think it's a breaking change, originally users send hash 0x1234 now {"blockHash": "0x1234"} is required. But it's a debug API anyway, it's totally fine to change it.

No it's not a breaking change. rpc.BlockNumberOrHash's unmarshaling rules are flexible. It accepts both {blockNumber: '', blockHash: ''} as well as '0x1' or 0xhash. debug.traceCall takes this as parameter. That's what I tested with.

> debug.traceCall({}, '0x61276fed11a014a7e2b04957033815f385931098c8179668612eb138e4b09683')
{
  failed: false,
  gas: 53000,
  returnValue: "",
  structLogs: []
}

@holiman holiman merged commit d789c68 into ethereum:master May 30, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@jwasinger @holiman
@s1na @devopsbo3
eth: make debug_StorageRangeAt take a block hash or number (ethereum#…
52775d5 
…27328)

eth: make StorageRangeAt take a block hash or number

Co-authored-by: Martin Holst Swende <[email protected]>
Co-authored-by: Sina Mahmoodi <[email protected]>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "eth: make debug_StorageRangeAt take a block hash or number (e…
352d68e 
…thereum#27328)"

This reverts commit 52775d5.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "eth: make debug_StorageRangeAt take a block hash or number (e…
c0fe114 
…thereum#27328)"

This reverts commit 52775d5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MariusVanDerWijden MariusVanDerWijden MariusVanDerWijden approved these changes

@rjl493456442 rjl493456442 rjl493456442 approved these changes

@s1na s1na s1na approved these changes

@holiman holiman holiman approved these changes

@karalabe karalabe Awaiting requested review from karalabe karalabe is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.1
Development

Successfully merging this pull request may close these issues.
5 participants
@jwasinger @holiman @s1na @rjl493456442 @MariusVanDerWijden