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: well-formed JSON error responses on /v2/fees/transaction #5793

Merged
merged 11 commits into from
Feb 5, 2025

Conversation

jcnelson
Copy link
Member

@jcnelson jcnelson commented Feb 4, 2025

Fixes #4145 regression in develop. Now, the fee estimator endpoint returns the JSON errors expected by stacks.js.

@jcnelson jcnelson requested review from a team as code owners February 4, 2025 03:08
@jcnelson jcnelson changed the base branch from master to develop February 4, 2025 03:08
zone117x
zone117x previously approved these changes Feb 4, 2025
Copy link
Member

@zone117x zone117x left a comment

Choose a reason for hiding this comment

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

The test case showing correct json response for the NoEstimateAvailable case looks good 👍

@kantai kantai changed the title Fix/4145 Fix: Return well-formed JSON error responses on /v2/fees/transaction Feb 4, 2025
@kantai kantai changed the title Fix: Return well-formed JSON error responses on /v2/fees/transaction Fix: well-formed JSON error responses on /v2/fees/transaction Feb 4, 2025
@kantai kantai added this to the 3.1.0.0.6 milestone Feb 4, 2025
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

Can you add a changelog entry?

obycode
obycode previously approved these changes Feb 4, 2025
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM, just noticed one typo.

stackslib/src/net/api/tests/postfeerate.rs Outdated Show resolved Hide resolved
@jcnelson jcnelson dismissed stale reviews from obycode and zone117x via b4af50d February 4, 2025 18:49
obycode
obycode previously approved these changes Feb 4, 2025
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

This looks good, but you're going to have to fix a merge conflict in the Changelog when #5798 merges shortly.

CHANGELOG.md Outdated Show resolved Hide resolved
zone117x
zone117x previously approved these changes Feb 4, 2025
@jcnelson jcnelson dismissed stale reviews from zone117x and obycode via 8ad0a2b February 5, 2025 01:47
@jcnelson jcnelson requested review from zone117x and obycode February 5, 2025 01:47
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

👍

@jcnelson jcnelson added this pull request to the merge queue Feb 5, 2025
Merged via the queue into develop with commit 9b48330 Feb 5, 2025
184 of 185 checks passed
@jcnelson jcnelson deleted the fix/4145 branch February 5, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Regression in next branch RPC endpoints
4 participants