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(api-kit): Missing to parameter #1058

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Conversation

yagopv
Copy link
Member

@yagopv yagopv commented Nov 22, 2024

What it solves

Resolves #1046

How this PR fixes it

Adding the to parameter to the decodeData method

@yagopv yagopv requested a review from DaniSomoza November 22, 2024 08:16
@coveralls
Copy link

coveralls commented Nov 22, 2024

Pull Request Test Coverage Report for Build 12008535981

Details

  • 2 of 3 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.06%) to 75.458%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/api-kit/src/SafeApiKit.ts 2 3 66.67%
Totals Coverage Status
Change from base Build 12008523214: -0.06%
Covered Lines: 788
Relevant Lines: 973

💛 - Coveralls

return sendRequest({
url: `${this.#txServiceBaseUrl}/v1/data-decoder/`,
method: HttpMethod.Post,
body: { data }
body: dataDecoderRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simplify this to?:

body: to ? { to, data } : { data }

* @returns The transaction data decoded
* @throws "Invalid data"
* @throws "Not Found"
* @throws "Ensure this field has at least 1 hexadecimal chars (not counting 0x)."
*/
async decodeData(data: string): Promise<any> {
async decodeData(data: string, to?: string): Promise<any> {
Copy link
Contributor

@DaniSomoza DaniSomoza Nov 25, 2024

Choose a reason for hiding this comment

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

Can use this Type instead of Promise<any> ?

type DecodeParameterType = {
  name: string
  type: string
  value: string
}

type DecodeReturnType = {
  method: string
  parameters: DecodeParameterType[]
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see the correct type on the service:

https://safe-transaction-sepolia.staging.5afe.dev/#/data-decoder/data_decoder_create
https://github.com/safe-global/safe-transaction-service/blob/f446d55cb527358e5312cea5aa737e0bae1adc6c/safe_transaction_service/history/views.py#L1188-L1225

The return type i'm seeing does not match the { data, to } structure that is in reality the parameters. So not sure if that type is always going to be the one or it can change

@yagopv yagopv linked an issue Nov 25, 2024 that may be closed by this pull request
@yagopv yagopv merged commit e0317d5 into development Nov 25, 2024
19 checks passed
@yagopv yagopv deleted the fix/decode-data-endpoint branch November 25, 2024 11:20
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ApiKit Data Decoder Doesn't Accept "to"
3 participants