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

fix #1964 abi serialization for string-ascii and string-utf8 #1965

Merged
merged 1 commit into from Oct 14, 2020
Merged

fix #1964 abi serialization for string-ascii and string-utf8 #1965

merged 1 commit into from Oct 14, 2020

Conversation

psq
Copy link
Contributor

@psq psq commented Oct 14, 2020

cc @zone117x just an FYI, nothing to do on the stacks-blockchain-api side, but the failure was happening there when calling a contract function with a string parameter, as described in #1964. Kind of surprising no one has encountered this before.

@psq psq requested a review from lgalabru October 14, 2020 06:51
@psq psq self-assigned this Oct 14, 2020
@project-bot project-bot bot added this to Review in progress in Stacks Blockchain Board Oct 14, 2020
@psq
Copy link
Contributor Author

psq commented Oct 14, 2020

this should not be a problem to merge this into master, this should only be affecting the rpc api

Stacks Blockchain Board automation moved this from Review in progress to Reviewer approved Oct 14, 2020
lgalabru
lgalabru approved these changes Oct 14, 2020
View reviewed changes
Copy link
Member

@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you @psq!

@kantai
Copy link
Member

kantai commented Oct 14, 2020 •
edited

@yknl -- does a change in this ABI serialization impact clarity-js-sdk or cli-blockstack?

This also seems like it would impact the sidecar, should we open an issue for that @zone117x ?

EDIT: Oh, I see, this was opened in response to the serialization already being out-of-sync.

kantai
kantai approved these changes Oct 14, 2020
View reviewed changes
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM!

@kantai kantai merged commit 201cedb into stacks-network:master Oct 14, 2020
Stacks Blockchain Board automation moved this from Reviewer approved to Done Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgalabru lgalabru lgalabru approved these changes

@kantai kantai kantai approved these changes

Assignees

@psq psq

Labels
None yet
Projects
Stacks Blockchain Board
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@psq @kantai @lgalabru