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.

Sign up for GitHub

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

Jump to bottom

Wrong user-facing encoding for poold id and reward account #2023

Closed
2 tasks done
KtorZ opened this issue Aug 11, 2020 · 6 comments
Closed
2 tasks done

Wrong user-facing encoding for poold id and reward account #2023

KtorZ opened this issue Aug 11, 2020 · 6 comments
Assignees
@hasufell
Labels
Bug PRIORITY:MEDIUM A bug that needs to be addressed after ongoing stories and tasks. SEVERITY:LOW Small defects which does not prevent any crucial functionality to work.

Comments

@KtorZ
Copy link
Member

KtorZ commented Aug 11, 2020 •
edited by rhyslbw
Loading

Context

Information -
Version
Platform
Installation
  • Pool id are currently encoded as base16 strings.
  • Reward accounts are currently encoded as bech32 with a "stake_addr" prefix

Steps to Reproduce

N/A

Expected behavior

  • Both types are encoded as bech32 using prefixes as specified in CIP-5

Actual behavior

Resolution

QA
@KtorZ KtorZ added this to Backlog in Adrestia via automation Aug 11, 2020
@KtorZ KtorZ added Bug SEVERITY:LOW Small defects which does not prevent any crucial functionality to work. PRIORITY:MEDIUM A bug that needs to be addressed after ongoing stories and tasks. and removed BUG:MINOR labels Aug 12, 2020
@hasufell hasufell self-assigned this Aug 21, 2020
@hasufell
Copy link
Contributor

hasufell commented Aug 21, 2020 •
edited
Loading

Where in the API can I reproduce this?

Edit:

Any other endpoints affected?

The offending code spots are these, afais:

https://github.com/input-output-hk/cardano-wallet/blob/d7994e9c83ed00d798cf24f3101e50f0383ddc17/lib/shelley/src/Cardano/Wallet/Shelley/Compatibility.hs#L921-L931

https://github.com/input-output-hk/cardano-wallet/blob/d7994e9c83ed00d798cf24f3101e50f0383ddc17/lib/core/src/Cardano/Wallet/Primitive/Types.hs#L677-L695

@hasufell hasufell moved this from Backlog to QA in Adrestia Aug 21, 2020
@hasufell hasufell moved this from QA to In Progress in Adrestia Aug 21, 2020
@hasufell
Copy link
Contributor

hasufell commented Aug 21, 2020 •
edited
Loading

Afais this will change various API endpoints, such as: https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/joinStakePool where the pool ID is a parameter

Having the encoding for parameter and json output diverge is probably going to be confusing?

hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 21, 2020
@hasufell
Fix bech32 prefixes for reward accounts wrt cardano-foundation#2023
aae53a6
hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 21, 2020
@hasufell
Encode PoolId as Bech32 instead of Base16 wrt cardano-foundation#2023
8be6b63
hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 21, 2020
@hasufell
Encode PoolId as Bech32 instead of Base16 wrt cardano-foundation#2023
ca0f2bb
hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 21, 2020
@hasufell
Encode PoolId as Bech32 instead of Base16 wrt cardano-foundation#2023
f563f17
iohk-bors bot added a commit that referenced this issue Aug 27, 2020
@iohk-bors @hasufell
Merge #2055
4decaf5 
2055: Fix user facing encoding wrt #2023 r=hasufell a=hasufell

So I kept the legacy parsing for both json and the query params to not break API for existing users.

Co-authored-by: Julian Ospald <julian.ospald@iohk.io>
Co-authored-by: Julian Ospald <hasufell@posteo.de>
hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 27, 2020
@hasufell
Fix bech32 prefixes for reward accounts wrt cardano-foundation#2023
c36ba7d
hasufell added a commit to hasufell/cardano-wallet that referenced this issue Aug 27, 2020
@hasufell
Encode PoolId as Bech32 instead of Base16 wrt cardano-foundation#2023
e96b26b
iohk-bors bot added a commit that referenced this issue Aug 27, 2020
@iohk-bors @hasufell
Merge #2055
8a1c0cd 
2055: Fix user facing encoding wrt #2023 r=hasufell a=hasufell

So I kept the legacy parsing for both json and the query params to not break API for existing users.

Co-authored-by: Julian Ospald <julian.ospald@iohk.io>
@piotr-iohk piotr-iohk moved this from In Progress to QA in Adrestia Aug 31, 2020
@piotr-iohk piotr-iohk mentioned this issue Aug 31, 2020
Merged
@piotr-iohk
Copy link
Contributor

lgtm.

I have adjusted swagger to show that pool id can be hex or bech32 in #2093 which should also address #2025.

iohk-bors bot added a commit that referenced this issue Sep 1, 2020
@iohk-bors
Merge #2093
406c35f 
2093: Swagger adjustment: Supported pool id param is hex or bech32 r=piotr-iohk a=piotr-iohk

# Issue Number

#2023
#2025

# Overview

- 01d5d50
  Supported pool id param is hex or bech32



# Comments

**Before:**

![Screenshot from 2020-08-31 13-42-59](https://user-images.githubusercontent.com/42900201/91716765-8c4e9680-eb90-11ea-89c3-4c368b4a9012.png)


**After:**

![Screenshot from 2020-08-31 16-16-37](https://user-images.githubusercontent.com/42900201/91729837-6d0e3400-eba5-11ea-8154-960e330e46ac.png)






Co-authored-by: Piotr Stachyra <piotr.stachyra@iohk.io>
@piotr-iohk piotr-iohk moved this from QA to Closed in Adrestia Sep 1, 2020
@rhyslbw rhyslbw reopened this Sep 25, 2020
Adrestia automation moved this from Closed to In Progress Sep 25, 2020
@hasufell
Copy link
Contributor

@rhyslbw what's missing?

@rhyslbw
Copy link
Contributor

rhyslbw commented Sep 28, 2020

Reward accounts are currently encoded as bech32 with a "stake_addr" prefix

@rhyslbw
Copy link
Contributor

rhyslbw commented Sep 28, 2020

Done in c36ba7d

@rhyslbw rhyslbw closed this as completed Sep 28, 2020
Adrestia automation moved this from In Progress to Closed Sep 28, 2020
@CharlesMorgan-IOHK CharlesMorgan-IOHK removed this from Closed in Adrestia Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hasufell hasufell

Labels
Bug PRIORITY:MEDIUM A bug that needs to be addressed after ongoing stories and tasks. SEVERITY:LOW Small defects which does not prevent any crucial functionality to work.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rhyslbw @hasufell @KtorZ @piotr-iohk