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

Feat/support pw transfer #1928

Merged
merged 7 commits into from Dec 7, 2020
Merged

Feat/support pw transfer #1928

merged 7 commits into from Dec 7, 2020

Conversation

katat
Copy link
Contributor

@katat katat commented Nov 25, 2020

  • support transfer to pw lock
  • remove the front end check for code hash since the backend also does the check.

@katat katat changed the base branch from hotfix/0.33.2 to develop December 1, 2020 03:04
@@ -29,10 +28,6 @@ export const validateSUDTAddress = ({
}

const CODE_HASH_LENGTH = 64
const codeHashOfAddr = parsed.substr(4, CODE_HASH_LENGTH)
if (codeHash && codeHashOfAddr !== codeHash.slice(2)) {
throw new FieldInvalidException(FIELD_NAME)
Copy link
Collaborator

Choose a reason for hiding this comment

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

By removing the validation of code hash, the error message for invalid code hash will be shown at the top but not beneath the input field. Is that expected?

Copy link
Contributor Author

@katat katat Dec 7, 2020

Choose a reason for hiding this comment

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

Instead of Invalid code hash, it should throw Invalid anyone-can-pay lock script address at the top, which would be better renamed as Unsupported anyone-can-pay lock script address

I think it is fine since the error only throws when the code hash is not supported, which is rare in practice. It might not be ideal, but it'd better avoid duplicating this code hash check at both front end and back end.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it.

I prefer checking it at front end for user experience and back end for security, but it does attenuate duplication of work to validate it at the back end only.

@katat katat merged commit 863b676 into develop Dec 7, 2020
@kellyshang kellyshang deleted the feat/support-pw-transfer branch October 26, 2021 13:56
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

3 participants