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

pv: add string corehash transform #1218

Merged
merged 1 commit into from Aug 17, 2017
Merged

pv: add string corehash transform #1218

merged 1 commit into from Aug 17, 2017

Conversation

lazedo
Copy link
Contributor

@lazedo lazedo commented Aug 17, 2017

Type Of Change

  • Small bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would change existing functionality)

Checklist:

  • PR should be backported to stable branches
  • Tested changes locally
  • Related to issue #XXXX (replace XXXX with an open issue number)

Description

adds string transform with optional size parameter to return the slot if supplied

usage

  • $(var(myvar){s.corehash}) returns the hash
  • $(var(myvar){s.corehash, 32}) returns the slot

would be great if this can be backported to branch 5.0 and included in next week release

optional size parameter to return the slot if supplied
example $(var(myvar){s.corehash}) returns the hash
        $(var(myvar){s.corehash, 32}) returns the slot
@miconda
Copy link
Member

miconda commented Aug 17, 2017

Thanks!

@miconda miconda merged commit 38c8597 into master Aug 17, 2017
@miconda
Copy link
Member

miconda commented Aug 17, 2017

I merged, then pushed some adjustments.

What I noticed is that the value is returned as a string, while one may expect a number, given that's typically an hash id. Any particular reason for this?

@lazedo
Copy link
Contributor Author

lazedo commented Aug 17, 2017

@miconda thanks. the reason was for large numbers not handled properly in script (uint) afaik.

@lazedo lazedo deleted the corehash_trans branch August 17, 2017 17:39
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