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

[ADP-3359] Report unaccepted-era transactions as HTTP 403 errors #4595

Merged

Conversation

paolino
Copy link
Collaborator

@paolino paolino commented May 16, 2024

  • Remove an error call when not-in-a-recent-era tx is posted
  • Add a 403 HTTP error in case the exception above is triggered

ADP-3359

@paolino paolino added the Conway PRs to prepare to the Conway bump label May 16, 2024
@paolino paolino self-assigned this May 16, 2024
Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

When adding UnsupportedEraInTxSubmission you need to add its counterparts in swagger, so something like :

x-errUnsupportedEraInTxSubmission: &errUnsupportedEraInTxSubmission                                                                                                                                                                  
   <<: *responsesErr                                                                                                                                                                                           
   title: unsupported_era_in_tx_submission                                                                                                                                                                                  
   properties:                                                                                                                                                                                                               message:                                                                                                                                                                                                  
       type: string                                                                                                                                                                                            
       description: |                                                                                                                                                                                          
         Happens when transaction was constructed before Babbage era                                                                                                                                                  
     code:                                                                                                                                                                                                     
       type: string                                                                                                                                                                                            
       enum: ['unsupported_era_in_tx_submission']

And insert errUnsupportedEraInTxSubmission under proper submit endpoint in 403 hierarchy.
Without it unit test will fail as there is no correspondence between ApiError and swagger errors.

Also, what about adding integration test with Alonzo tx as a cbor and showing that the endpoint endup with 403 and proper error msg?

@paolino paolino force-pushed the paolino/ADP-3359/report-unacceptet-era-tx-as-403 branch 5 times, most recently from 2d69876 to 638da7f Compare May 20, 2024 13:03
@paolino paolino force-pushed the paolino/ADP-3359/report-unacceptet-era-tx-as-403 branch from 638da7f to 345f905 Compare May 20, 2024 14:51
Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

Hi @paolino
Many thanks for making this PR.
I've left some suggestions (and one small request)!

lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs Outdated Show resolved Hide resolved
lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs Outdated Show resolved Hide resolved
lib/api/src/Cardano/Wallet/Api/Types/Error.hs Show resolved Hide resolved
specifications/api/swagger.yaml Show resolved Hide resolved
@paolino paolino force-pushed the paolino/ADP-3359/report-unacceptet-era-tx-as-403 branch from 345f905 to bf3471a Compare May 21, 2024 15:49
Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

nice! I have added integration tests showing that your changes handle tx constructed in previous eras as it is expected .

@jonathanknowles jonathanknowles added this pull request to the merge queue May 22, 2024
Merged via the queue into master with commit 1a0ea67 May 22, 2024
4 checks passed
@jonathanknowles jonathanknowles deleted the paolino/ADP-3359/report-unacceptet-era-tx-as-403 branch May 22, 2024 10:02
WilliamKingNoel-Bot pushed a commit that referenced this pull request May 22, 2024
#4595) - [x] Remove an `error` call when not-in-a-recent-era tx is posted - [x] Add a 403 HTTP error in case the exception above is triggered ADP-3359 Source commit: 1a0ea67
github-merge-queue bot pushed a commit that referenced this pull request May 22, 2024
…s. (#4603)

This PR:
- adds a new constant `recentEras :: Set AnyRecentEra`
- this constant defines the complete set of **_recent eras_** that are
supported for transaction construction.
- uses this constant to simplify the construction of the following API
errors:
    - `ApiErrorNodeNotYetInRecentEra`
    - `ApiErrorUnsupportedEra`

## Issue

ADP-3359

Follow-on from #4595.
github-merge-queue bot pushed a commit that referenced this pull request May 23, 2024
…e. (#4605)

This PR moves the `ApiEra` type and related functions to a separate
module `Api.Types.Era`.

As a result:
- the `ApiEra` type is co-located only with its related functions;
- the `Api.Types`
[megamodule](https://www.parsonsmatt.org/2019/11/27/keeping_compilation_fast.html)
is now ever-so-slightly smaller.

Additionally:
- functions relating to `ApiEra` are now imported via the `ApiEra`
qualifier;
- the `Api.Types` module does **_not_** re-export symbols from
`Api.Types.Era`.

## Issue

ADP-3359

Follow-on from #4595.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Conway PRs to prepare to the Conway bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants