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

On-chain function to convert from Integer to ByteString, or to parse ByteString into Integer #3657

Closed
1 of 4 tasks
longngn opened this issue Jul 31, 2021 · 16 comments
Closed
1 of 4 tasks
Assignees
Labels
enhancement Low priority Doesn't require immediate attention status: triaged

Comments

@longngn
Copy link

longngn commented Jul 31, 2021

Area

  • Plutus Foundation Related to the GHC plugin, Haskell-to-Plutus compiler, on-chain code
  • Plutus Application Framework Related to the Plutus application backend (PAB), emulator, Plutus libraries
  • Marlowe Related to Marlowe
  • Other Any other topic (Playgrounds, etc.)

Describe the feature you'd like

  • On-chain function to convert from Integer (or IsData instance) to ByteString
  • On-chain function to parse ByteString into Integer (or IsData instance)

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context / screenshots

Add any other context or screenshots about the feature request here.

@catch-21
Copy link
Contributor

catch-21 commented Aug 2, 2021

Hi, thanks @longngn. This is actually already being worked on to allow for Marlowe scalability improvements. I'll link a PR once there's one ready.

@catch-21 catch-21 self-assigned this Aug 2, 2021
@longngn
Copy link
Author

longngn commented Aug 4, 2021

Hi, thanks @longngn. This is actually already being worked on to allow for Marlowe scalability improvements. I'll link a PR once there's one ready.

Thank James for working on this feature! Is it just serialize/deserialize Integer or any arbitrary IsData instance?

@L-as
Copy link
Contributor

L-as commented Oct 29, 2021

This might also fix #4168 .

Is there any update on this?

@longngn
Copy link
Author

longngn commented Oct 29, 2021

Actually I've written an Integer -> BS function for our internal contracts. Happy to write the reverse direction (BS -> Integer) and property-based testing and to create a PR

-- Convert from an integer to its text representation. Example: 123 => "123"
{-# INLINEABLE integerToBS #-}
integerToBS :: Integer -> BuiltinByteString
integerToBS x
  -- 45 is ASCII code for '-'
  | x < 0 = consByteString 45 $ integerToBS (negate x)
  -- x is single-digit
  | x `quotient` 10 == 0 = digitToBS x
  | otherwise = integerToBS (x `quotient` 10) <> digitToBS (x `remainder` 10)
  where
    digitToBS :: Integer -> BuiltinByteString
    -- 48 is ASCII code for '0'
    digitToBS d = consByteString (d + 48) emptyByteString

@L-as
Copy link
Contributor

L-as commented Oct 29, 2021

Thanks! Ah, you meant ASCII? I thought this issue was for getting a binary representation of it.

@Benjmhart
Copy link

+1 for binary representation as we're trying to enable efficient cryptographic behaviors.

@michaelpj
Copy link
Contributor

Thanks! Ah, you meant ASCII? I thought this issue was for getting a binary representation of it.

As you point out here, there is no unique Integer -> ByteString mapping. You have to pick an encoding. So any builtin that we picked would be for a particular encoding. So we'd need to pick a particular one.

@L-as
Copy link
Contributor

L-as commented Nov 25, 2021 via email

@michaelpj
Copy link
Contributor

No, there are many:

  • Big/little-endian
  • MSB/LSB-first

@L-as
Copy link
Contributor

L-as commented Nov 26, 2021

Little-endian is probably best because that's probably what most of us are used to dealing with.

@longngn
Copy link
Author

longngn commented Nov 26, 2021

Perhaps we can use ZigZag encoding to encode variable-length signed integer like in Plutus Core
Base-256 is only possible for unsigned integer

@effectfully
Copy link
Contributor

effectfully commented Feb 22, 2023

Describe the feature you'd like

On-chain function to convert from Integer (or IsData instance) to ByteString

We now have

  1. serialiseData :: BuiltinData -> BuiltinByteString
  2. a Show Builtins.Integer instance that you can use together with encodeUtf8

I don't know if we have any parsing capabilities.

@zliu41 do you happen to have any input on this issue?

@zliu41
Copy link
Member

zliu41 commented Feb 22, 2023

Requests for adding new builtin functions should be discussed in https://github.com/cardano-foundation/CIPs.

For Plutus Tx library functions, my main concern is that converting integers to/from bytestrings without using new builtins, regardless of encoding, can be quite expensive (perhaps except serialiseData . mkI). I don't want to add a library function and make users think that they can just use it casually in their scripts (we do have the Show class, but it is intended for debugging). If we make it clear it is expensive, maybe it is OK.

@effectfully effectfully added the status: needs triage GH issues that requires triage label Apr 10, 2023
@effectfully
Copy link
Contributor

To iterate on what @michaelpj and @zliu41 said, the choices that we have are:

  1. add new builtins (tailored ones, or just deserialiseData). This should be done via https://github.com/cardano-foundation/CIPs
  2. implement encoding/decoding as regular functions by making arbitrary choices regarding the encoding. Those functions aren't going to be efficient and we're worried about having to handle complaints, optimize the functions and generally spend a lot of time on it. In particular, just looking at @longngn implementation (thank you @longngn for providing it!) from the above I can spot a number of performance issues right away: lack of tail recursion, quadratic behavior due to <> and consByteString being linear (which we probably can't avoid), x `quotient` 10 computed twice, x < 0 unnecessarily computed at each step, maybe something else -- not to mention that this direct encoding is nicely visual, but not compact at all

So given the lack of consensus and what appears to be a low priority problem, I'm going to label the issue with "Low priority".

@effectfully effectfully added Low priority Doesn't require immediate attention status: triaged and removed status: needs triage GH issues that requires triage labels Jun 20, 2023
@effectfully
Copy link
Contributor

or just deserialiseData

Incidentally, here is some context regarding why we don't have this builtin. Credit for digging that out goes to @kwxm.

@effectfully
Copy link
Contributor

There now exist the integerToByteString and byteStringToInteger builtins, so this issue is resolved and I'm closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Low priority Doesn't require immediate attention status: triaged
Projects
None yet
Development

No branches or pull requests

7 participants