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 balances query endpoint cost without indexation and behavior coins to spend with one parameter at zero #2662

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

AurelienFT
Copy link
Contributor

Description

The two changes has been packed in one PR to speed up the release process given our deadlines.

The balance query endpoint cost if useful for the TSSDK team.
The coins to spend behaviour change roll back to the previous behavior, asked by Rust SDK team

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

rymnc
rymnc previously approved these changes Feb 3, 2025
Copy link
Member

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

wondering about the complexity change, but if tests pass lgtm

@AurelienFT AurelienFT enabled auto-merge (squash) February 3, 2025 16:02
@AurelienFT AurelienFT mentioned this pull request Feb 3, 2025
netrome
netrome previously approved these changes Feb 3, 2025
@netrome
Copy link
Contributor

netrome commented Feb 3, 2025

wondering about the complexity change, but if tests pass lgtm

Also wondering about this.

Comment on lines +100 to +103
#[graphql(complexity = "if query_costs().balance_query == 0 { \
(child_complexity as f32 * first.unwrap_or_default() as f32 * 0.66) as usize + \
(child_complexity as f32 * last.unwrap_or_default() as f32 * 0.66) as usize
")]
} else { query_costs().balance_query }")]
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather it be explicit:

if indexation_set {
    query_costs().balance_query
} else {
    ...
}

If that's not possible, let's leave a clear comment explaining the relationship between 0 and indexation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@AurelienFT AurelienFT dismissed stale reviews from netrome and rymnc via 77d0e79 February 3, 2025 19:39
@AurelienFT AurelienFT requested review from rymnc and netrome February 3, 2025 19:40
@AurelienFT AurelienFT merged commit e342cd7 into master Feb 3, 2025
32 checks passed
@AurelienFT AurelienFT deleted the fix_api_sdk_issues branch February 3, 2025 19:58
AurelienFT added a commit that referenced this pull request Feb 3, 2025
## Version 0.41.5

### Changed
- [2387](#2387): Update
description `tx-max-depth` flag.
- [2630](#2630): Removed some
noisy `tracing::info!` logs
- [2643](#2643): Before this
fix when tip is zero, transactions that use 30M have the same priority
as transactions with 1M gas. Now they are correctly ordered.

### Added
- [2617](#2617): Add
integration skeleton of parallel-executor.
- [2553](#2553): Scaffold
global merkle root storage crate.
- [2598](#2598): Add initial
test suite for global merkle root storage updates.
- [2635](#2635): Add metrics
to gas price service

### Fixed
- [2632](#2632): Improved
performance of certain async trait impls in the gas price service.
- [2662](#2662): Fix balances
query endpoint cost without indexation and behavior coins to spend with
one parameter at zero.
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