Skip to content

Reintroduce missing 4844 RPC fields (GasPrice and V)#8232

Merged
LukaszRozmej merged 4 commits into
masterfrom
fix/missing-4844-rpc-fields
Feb 20, 2025
Merged

Reintroduce missing 4844 RPC fields (GasPrice and V)#8232
LukaszRozmej merged 4 commits into
masterfrom
fix/missing-4844-rpc-fields

Conversation

@LukaszRozmej

@LukaszRozmej LukaszRozmej commented Feb 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #8230 introduced in #7483

Changes

  • Reintroduce missing 4844 RPC fields (GasPrice and V)

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Potentially hotfix release?

@LukaszRozmej

LukaszRozmej commented Feb 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Looking at https://ethereum.github.io/execution-apis/api-documentation/ it still should be there.

:ooking at spec it is not required field, but not restricted
https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml
https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L140-L143
https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L372-L375

emlautarom1
emlautarom1 requested changes Feb 20, 2025
View reviewed changes

@emlautarom1 emlautarom1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We most likely also want to do the same with SetCode transactions:

nethermind/src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/SetCodeTransactionForRpc.cs

Lines 20 to 26 in b9ed459

#region Deprecated fields
[JsonIgnore(Condition = JsonIgnoreCondition.Always)]
public override UInt256? GasPrice { get; set; }
[JsonIgnore(Condition = JsonIgnoreCondition.Always)]
public override UInt256? V { get; set; }
#endregion

nethermind/src/Nethermind/Nethermind.JsonRpc.Test/Modules/RpcTransaction/SetCodeTransactionForRpcTests.cs

Lines 133 to 135 in b9ed459

// Assert deprecated fields are no longer serialized
json.TryGetProperty("gasPrice", out _).Should().BeFalse();
json.TryGetProperty("v", out _).Should().BeFalse();

@emlautarom1 emlautarom1 mentioned this pull request Feb 20, 2025
Closed
@LukaszRozmej LukaszRozmej merged commit 86a7e0f into master Feb 20, 2025
@LukaszRozmej LukaszRozmej deleted the fix/missing-4844-rpc-fields branch February 20, 2025 23:25
LukaszRozmej added a commit that referenced this pull request Feb 27, 2025
@LukaszRozmej
Reintroduce missing 4844 RPC fields (GasPrice and V) (#8232)
67c1cb4
@emlautarom1 emlautarom1 mentioned this pull request May 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarekM25 MarekM25 MarekM25 approved these changes

@emlautarom1 emlautarom1 emlautarom1 approved these changes

Assignees

No one assigned

Labels

bug fix + reliability

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing gasPrice and v fields in eth_getTransactionByHash response on Nethermind 1.30.3

4 participants

@LukaszRozmej @MarekM25 @emlautarom1 @benaadams