Skip to content

Backport of PR #771 to 0.34 : big int parsing#798

Merged
jmalicevic merged 9 commits intov0.34.xfrom
jasmina/0.34-bigint
May 11, 2023
Merged

Backport of PR #771 to 0.34 : big int parsing#798
jmalicevic merged 9 commits intov0.34.xfrom
jasmina/0.34-bigint

Conversation

@jmalicevic
Copy link
Collaborator

Closes #726 for 0.34.x

This is a manual backport of #771 because Mergify did not trigger it.

jmalicevic and others added 3 commits May 5, 2023 13:01
Replaced int64 with big.int

Co-authored-by: Lasaro <lasaro@informal.systems>
Co-authored-by: Sergio Mena <sergio@informal.systems>
@jmalicevic jmalicevic added bug Something isn't working indexer rpc labels May 5, 2023
@jmalicevic jmalicevic self-assigned this May 5, 2023
@jmalicevic jmalicevic requested a review from a team as a code owner May 5, 2023 11:22
@jmalicevic jmalicevic marked this pull request as draft May 5, 2023 11:27
@jmalicevic jmalicevic marked this pull request as ready for review May 8, 2023 09:21
Copy link
Collaborator

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

Only minor comments on v0.34.x <--> v0.37.x discrepancies. I'm interested in the reasons for those

jmalicevic and others added 2 commits May 8, 2023 23:50
Copy link
Collaborator

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

LGTM.
Please make sure you address @lasarojc's comments

Comment on lines +1 to +2
- `[state/kvindex]` Querying event attributes that are bigger than int64 is now enabled. We are not supporting reading floats from the db into the indexer nor parsing them into BigFloats to not introduce breaking changes in minor releases.
([\#771](https://github.com/cometbft/cometbft/pull/771)) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Querying event attributes that are bigger than int64 is now enabled.

This makes sense to me as something to communicate to users.

We are not supporting reading floats from the db into the indexer nor parsing them into BigFloats to not introduce breaking changes in minor releases.

What does this, however, mean for users?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe this does not have to be in the changelog, but only in the docs. This is basically the same behaviour as they had before, but they might have been unaware of this. I agree that maybe this should be removed frm the changelog but left in the docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, let me know if you are ok with merging this.

jmalicevic and others added 2 commits May 10, 2023 12:25
Co-authored-by: Thane Thomson <connect@thanethomson.com>
@jmalicevic jmalicevic merged commit 30af8c2 into v0.34.x May 11, 2023
@jmalicevic jmalicevic deleted the jasmina/0.34-bigint branch May 11, 2023 10:14
@jmalicevic
Copy link
Collaborator Author

@thanethomson , I am closing this as the content is in the docs, but feel free to ping me to change the changelog message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working indexer rpc

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants