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

Fix cursor block height decoding in SortedTXCursor #1950

Merged
merged 5 commits into from
Jun 11, 2024

Conversation

AurelienFT
Copy link
Contributor

#1825

The BlockHeight was encoded in hexadecimal in response of transactions endpoint and decoded with u32::from_str in parameter of type SortedTXCursor which was preventing from making chained API calls.
This PR, fixes this by decoding BlockHeight from hexadecimal form in SortedTXCursor.

This bug wasn't catch by the test tx::get_transactions() because the test was using only txs included in block 3 that has a string representation that is the same in an hexadecimal and decimal encoding. I added the creation of 10 blocks at start of the test so that now the txs are included in blocks that have a hexadecimal and decimal encoding form that differs.

I didn't spotted any hangs on the endpoint when the cursor is initialized manually, however as we can't ask to start at a precise block height without specifying a previous tx_id it's not very usable to make manual search.

PS: This is my first contribution and so to what I saw my PR doesn't brake anything else and I tried to follow the guidelines of the repository, however maybe their things that I missed and so I'm very open to criticise !

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog
  • New behavior is reflected in tests
  • The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

@CLAassistant
Copy link

CLAassistant commented Jun 9, 2024

CLA assistant check
All committers have signed the CLA.

@AurelienFT AurelienFT changed the title Fix cursor block height encoding transactions endpoint Fix cursor block height encoding in SortedTXCursor Jun 9, 2024
@AurelienFT AurelienFT changed the title Fix cursor block height encoding in SortedTXCursor Fix cursor block height decoding in SortedTXCursor Jun 10, 2024
Copy link
Member

@Voxelot Voxelot left a comment

Choose a reason for hiding this comment

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

LGTM - Thank you!

@Voxelot Voxelot merged commit 997c02b into FuelLabs:master Jun 11, 2024
30 of 33 checks passed
@xgreenx xgreenx mentioned this pull request Jun 14, 2024
xgreenx added a commit that referenced this pull request Jun 14, 2024
## Version v0.29.0

### Added
- [#1889](#1889): Add new
`FuelGasPriceProvider` that receives the gas price algorithm from a
`GasPriceService`

### Changed
- [#1942](#1942): Sequential
relayer's commits.
- [#1952](#1952): Change tip
sorting to ratio between tip and max gas sorting in txpool
- [#1960](#1960): Update
fuel-vm to v0.53.0.

### Fixed
- [#1950](#1950): Fix cursor
`BlockHeight` encoding in `SortedTXCursor`

## What's Changed
* Fix code coverage compilation and tests by @Dentosal in
#1943
* Weekly `cargo update` by @github-actions in
#1949
* Fix cursor block height decoding in SortedTXCursor by @AurelienFT in
#1950
* Sequential relayer's commits by @xgreenx in
#1942
* Add Gas Price Updater Service by @MitchTurner in
#1938
* Change tip sorting to ratio between tip and max gas sorting in txpool
by @AurelienFT in #1952
* deps(client): update eventsource-client to fix CVE(s) by @Br1ght0ne in
#1954
* Update fuel-vm to v0.53.0 by @Dentosal in
#1960

## New Contributors
* @AurelienFT made their first contribution in
#1950

**Full Changelog**:
v0.28.0...v0.29.0
GoldenPath1109 added a commit to GoldenPath1109/fuel-core that referenced this pull request Sep 7, 2024
## Version v0.29.0

### Added
- [#1889](FuelLabs/fuel-core#1889): Add new
`FuelGasPriceProvider` that receives the gas price algorithm from a
`GasPriceService`

### Changed
- [#1942](FuelLabs/fuel-core#1942): Sequential
relayer's commits.
- [#1952](FuelLabs/fuel-core#1952): Change tip
sorting to ratio between tip and max gas sorting in txpool
- [#1960](FuelLabs/fuel-core#1960): Update
fuel-vm to v0.53.0.

### Fixed
- [#1950](FuelLabs/fuel-core#1950): Fix cursor
`BlockHeight` encoding in `SortedTXCursor`

## What's Changed
* Fix code coverage compilation and tests by @Dentosal in
FuelLabs/fuel-core#1943
* Weekly `cargo update` by @github-actions in
FuelLabs/fuel-core#1949
* Fix cursor block height decoding in SortedTXCursor by @AurelienFT in
FuelLabs/fuel-core#1950
* Sequential relayer's commits by @xgreenx in
FuelLabs/fuel-core#1942
* Add Gas Price Updater Service by @MitchTurner in
FuelLabs/fuel-core#1938
* Change tip sorting to ratio between tip and max gas sorting in txpool
by @AurelienFT in FuelLabs/fuel-core#1952
* deps(client): update eventsource-client to fix CVE(s) by @Br1ght0ne in
FuelLabs/fuel-core#1954
* Update fuel-vm to v0.53.0 by @Dentosal in
FuelLabs/fuel-core#1960

## New Contributors
* @AurelienFT made their first contribution in
FuelLabs/fuel-core#1950

**Full Changelog**:
FuelLabs/fuel-core@v0.28.0...v0.29.0
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.

4 participants