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

[APIS-343] Fix fee coin selection in cosmos sign page #348

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

G-Gamja
Copy link
Member

@G-Gamja G-Gamja commented Jun 7, 2024

  • tx의 fee amount필드의 denom을 통해 fee 코인 변수를 선언할 때 체인리스트의 fee코인 내에서 선택되는게 아닌 tx의 fee.amount필드의 값이 선택되도록 수정했습니다.
    • tx의 fee denom이 체인 리스트의 fee코인 리스트에 포함되지 않았을때 생기는 오류를 해결했습니다.
      • raw tx의 fee코인과 UI상에서의 fee코인이 다른 문제
  • 사인 페이지에서 fee코인을 변경할 때 decimals가 변경되지 않는 오류를 수정했습니다.

- fee코인을 선택할 때 fee코인 내에서 선택되는게 아닌 tx의 fee.amount필드의 값이 선택되도록 수정했습니다.
- 사인 페이지에서 fee코인을 변경할 때 decimals가 변경되지 않는 오류를 수정했습니다.
@G-Gamja G-Gamja self-assigned this Jun 7, 2024
@G-Gamja G-Gamja marked this pull request as ready for review June 10, 2024 03:15
@G-Gamja G-Gamja requested a review from ong-ar as a code owner June 10, 2024 03:15
Copy link
Member

@ong-ar ong-ar left a comment

Choose a reason for hiding this comment

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

우리 리스트에 없더라도 사인은 하게 해주어야 하는데 currentFeeCoin 같은 경우 우리 리스트 없을 경우 amount 자체를 없다라고 되어있네요. 이부분은 수정이 필요할 거 같습니다.
우리 리스트에 없더라도 balance 에 해당 데놈에 대한 amount 는 있을 수 있습니다.
사인은 우리 앱 기준으로 쓸려고 만든게 아니라 dapp 에서 쓸려고 만든 거라 어떤 상황에서도 입력 값에 따른 시그니처는 내려줘야할 거 같습니다.

 - 체인리스트에 등록되지 않은 코인일지라도 fee코인으로 사용될 수 있기 때문에 무조건적으로 밸런스를 0으로 설정하는 것이 아닌 'balance' 리스폰스의 밸런스 값을 참조한 값이 설정되도록 변경했습니다.
@G-Gamja
Copy link
Member Author

G-Gamja commented Jun 13, 2024

우리 리스트에 없더라도 사인은 하게 해주어야 하는데 currentFeeCoin 같은 경우 우리 리스트 없을 경우 amount 자체를 없다라고 되어있네요. 이부분은 수정이 필요할 거 같습니다. 우리 리스트에 없더라도 balance 에 해당 데놈에 대한 amount 는 있을 수 있습니다. 사인은 우리 앱 기준으로 쓸려고 만든게 아니라 dapp 에서 쓸려고 만든 거라 어떤 상황에서도 입력 값에 따른 시그니처는 내려줘야할 거 같습니다.

리스트에 없는 fee 코인도 밸런스를 가져올 수 있도록 로직을 수정하겠습니다.
스크린샷 2024-06-13 오후 12 21 00

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.

None yet

2 participants