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

[Keymanager API] Support for the feerecipient end-points #3864

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

zah
Copy link
Contributor

@zah zah commented Jul 12, 2022

Other changes:

  • The Keymanager error responses differ from the Beacon API responses.
    'keymanagerApiError' replaces the former usages of 'jsonError'.

  • Return status code 401 and 403 for authorization errors in accordance
    to the spec.

  • Eliminate inconsistencies in the REST JSON parsing. Some of the code
    paths allowed missing fields.

  • Added logging of serialization failure details at DEBUG level.

Other changes:

* The Keymanager error responses differ from the Beacon API responses.
  'keymanagerApiError' replaces the former usages of 'jsonError'.

* Return status code 401 and 403 for authorization errors in accordance
  to the spec.

* Eliminate inconsistencies in the REST JSON parsing. Some of the code
  paths allowed missing fields.

* Added logging of serialization failure details at DEBUG level.
@zah zah force-pushed the keymanager-fee-recipient branch from b898dbb to c1fea49 Compare July 12, 2022 22:55
@github-actions
Copy link

Unit Test Results

       12 files  ±  0       860 suites  +3   1h 11m 3s ⏱️ + 5m 39s
  1 908 tests +  4    1 760 ✔️ +  4  148 💤 ±0  0 ±0 
10 349 runs  +21  10 143 ✔️ +21  206 💤 ±0  0 ±0 

Results for commit c1fea49. ± Comparison against base commit e0e7af7.

@zah zah merged commit 806536a into unstable Jul 13, 2022
@zah zah deleted the keymanager-fee-recipient branch July 13, 2022 14:45
etan-status added a commit that referenced this pull request Jul 13, 2022
#3864 introduced a regression by turning on `requireAllFields` globally
for JSON parsing. Certain endpoints such as `RestSyncInfo` have optional
fields that do not parse correctly without additional changes. As this
change was neither reviewed nor passed unit tests, reverting for now.
etan-status added a commit that referenced this pull request Jul 14, 2022
#3864 introduced a regression by turning on `requireAllFields` globally
for JSON parsing. Certain endpoints such as `RestSyncInfo` have optional
fields that do not parse correctly without additional changes. This is
reverted for now to restore previous behaviour and unblock CI testing.
etan-status added a commit that referenced this pull request Jul 14, 2022
#3864 introduced a regression by turning on `requireAllFields` globally
for JSON parsing. Certain endpoints such as `RestSyncInfo` have optional
fields that do not parse correctly without additional changes. This is
reverted for now to restore previous behaviour and unblock CI testing.
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.

1 participant