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

modified Block struct #873

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

modified Block struct #873

wants to merge 5 commits into from

Conversation

h33min
Copy link

@h33min h33min commented Oct 31, 2024

  • modified to allow an optional type of 'totalDifficulty'

  • According to 'ethereum/execution-apis@9e16d5e', you can see that the 'totalDifficulty field was removed from the Blockschema on September 4.

  • Since the existing structure required a 'totalDifficulty' field, if the result of a json structure without a 'totalDifficulty' field is returned from the Ethereum node, there is an issue that decoding fails in the web3swift library. To solve this problem, the 'totalDifficulty' field was changed to an optional field.

Summary of Changes

modified 'totalDifficulty' to optional type in Block struct

By submitting this pull request, you are confirming the following:
  • I have reviewed the Contribution Guidelines.
  • I have performed a self-review of my own code.
  • I have updated my repository to match the develop branch.
  • I have included test data or screenshots that prove my fix is effective or that my feature works.
  • I have checked that all tests work and swiftlint is not throwing any errors/warnings.

- modified to allow an optional type of 'totalDifficulty'
  - because 'totalDifficulty' was removed from Ethereum official Blockschema
@JeneaVranceanu JeneaVranceanu changed the base branch from develop to develop-4.0 October 31, 2024 07:25
Copy link
Collaborator

@JeneaVranceanu JeneaVranceanu 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 but this is a breaking change and we can only make it part of the v4 which I'm not sure when will be released.

If I make it target the develop-v4.0 branch than we have a problem:
Screenshot 2024-10-31 at 09 25 13

What I suggest is that we keep the totalDifficulty: BigUInt non optional and during decoding we provide a default value set to 0. We will add a comment alongside yours telling that 0 means this value is not defined and shall not be used.

Sources/Web3Core/Structure/Block/Block.swift Outdated Show resolved Hide resolved
Sources/Web3Core/Structure/Block/Block.swift Outdated Show resolved Hide resolved
@JeneaVranceanu JeneaVranceanu changed the base branch from develop-4.0 to develop October 31, 2024 07:31
@h33min
Copy link
Author

h33min commented Oct 31, 2024

Very Good Idea.

JeneaVranceanu
JeneaVranceanu previously approved these changes Nov 5, 2024
@yaroslavyaroslav yaroslavyaroslav force-pushed the develop branch 4 times, most recently from a0218ff to 8c152f5 Compare December 10, 2024 18:12
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.

2 participants