Add a builtin for decoding TokenMessage for hyperlane SPI by imalsogreg · Pull Request #1344 · kadena-io/pact

imalsogreg · 2024-02-21T19:19:50Z

Part of the SPI interface is a TokenMessage: a tuple of a recipient, and amount, and a chain id:

{
  amount: Decimal,
  chainId: Text,
  recipient: Guard
}

The wire format for a TokenMessage is a packed binary encoding of these fields, followed by base64url encoding.

Pact needs to provide a builtin for decoding such an encoded message into its original form, to support the larger SPI scheme. This PR implements that builtin, named hyperlane-decode-token-message. It also provides a convenience function for encoding a TokenMessage, useful for testing.

Usage:

pact>   (hyperlane-decode-token-message "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAewAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGF7InByZWQiOiAia2V5cy1hbGwiLCAia2V5cyI6WyJkYTFhMzM5YmQ4MmQyYzJlOTE4MDYyNmEwMGRjMDQzMjc1ZGViM2FiYWJiMjdiNTczOGFiZjZiOWRjZWU4ZGI2Il19AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==")
{"amount": 0.000000000000000123
,"chainId": "4"
,"recipient": KeySet {keys: [ da1a339bd82d2c2e9180626a00dc043275deb3ababb27b5738abf6b9dcee8db6 ]
,pred: keys-all}}

Todo:

The gas costs are chosen arbitrarily. Do benchmarking to find the correct cost.
Gate the builtin behind a version flag.

Gas analysis:

The following code was used to benchmark decoding:

benchTokenMessages :: [Benchmark]
benchTokenMessages = 
  let
    !msg1 = mkTokenMessage 1
    name1 = "tokenmessage-" ++ show (T.length msg1)
    !msg1000 = mkTokenMessage 1000
    name1000 = "tokenmessage-" ++ show (T.length msg1000)
    !msg2000 = mkTokenMessage 2000
    name2000 = "tokenmessage-" ++ show (T.length msg2000)
  in
  [ bench name1 $ whnf hyperlaneDecodeTokenMessage msg1
  , bench name1000 $ whnf hyperlaneDecodeTokenMessage msg1000
  , bench name2000 $ whnf hyperlaneDecodeTokenMessage msg2000
  ]

The results are: 2 microseconds for a 256-byte message, 11 microseconds for an 10880-byte message, 21 microseconds for a 21548-byte message.

400 MilliGas = 1 microsecond by policy. 10k bytes = 10 microseconds by benchmarks. 100 bytes = 0.1 microseconds = 40 MilliGas. We round up to 50 MilliGas as a safety buffer.

PR checklist:

Test coverage for the proposed changes
PR description contains example output from repl interaction or a snippet from unit test output
Documentation has been updated if new natives or FV properties have been added. To generate new documentation, issue cabal run tests. If they pass locally, docs are generated.
Any changes that could be relevant to users have been recorded in the changelog
In case of changes to the Pact trace output (pact -t), make sure pact-lsp is in sync.

Additionally, please justify why you should or should not do the following:

Confirm replay/back compat
Benchmark regressions
(For Kadena engineers) Run integration-tests against a Chainweb built with this version of Pact

src/Pact/Native.hs

pact.cabal

src/Pact/Native.hs

edmundnoble

My approval is waiting on @AK3N's

edmundnoble · 2024-02-21T22:56:25Z

pact.cabal

    , base >= 4.18.0.0
    , base16-bytestring >=0.1.1.6
    , base64-bytestring >=1.0.0.1
+    , binary


Needs a lower bound

edmundnoble · 2024-02-21T22:58:47Z

src/Pact/Native.hs

+
+      -- Parse the size of the following amount field.
+      amountSize <- fromIntegral @Word256 @Int <$> getWord256be
+      unless (amountSize == 96)


Why is this 96? Does @AK3N know?

That's not an amountSize but the offset of the recipient which is a string.

recipient is a first field of the TokenMessageERC20 structure and since it was encoded with abi.encode there is an offset:

0000000000000000000000000000000000000000000000000000000000000060 # offset of the recipient string = 96, because first three lines are 32 bytes each 0000000000000000000000000000000000000000000000008ac7230489e80000 # amount = 10000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 # chainId = 0 000000000000000000000000000000000000000000000000000000000000002a # recipientSize = 42 3078373143373635364543376162383862303938646566423735314237343031 # "0x71C7656EC7ab88b098defB751B7401B5f6d8976F" 4235663664383937364600000000000000000000000000000000000000000000

Let's leave this comment in the code

@AK3N Thanks for the comment example! The only part I don't understand is the example recipient "0x71C7656EC7ab88b098defB751B7401B5f6d8976F". Our recipients are now JSON-encoded guards, e.g. { "pred": "keys-any", "keys": [...] }. Should we replace the hex string with that?

ghost · 2024-02-22T09:40:25Z

src/Pact/Interpreter.hs


 pact411Natives :: [Text]
-pact411Natives = ["enforce-verifier", "hyperlane-message-id"]
+pact411Natives = ["enforce-verifier", "hyperlane-message-id", "hyperlane-decode-token-message"]


Either we need to specify the version hyperlane-v3-decode-token-message or pass an object with {"version": 3, ...} field.

@edmundnoble, should we also specify the type of the token? hyperlane-decode-token-message-erc20.

I don't think that's necessary, because tokenmessage is not actually part of the Hyperlane spec, so it's not versioned with it. We can make future versions, if they're needed, use -v2, -v3, etc.

src/Pact/Native.hs

ghost · 2024-02-22T09:54:49Z

src/Pact/Native.hs

+
+      -- Parse the size of the following amount field.
+      amountSize <- fromIntegral @Word256 @Int <$> getWord256be
+      unless (amountSize == 96)


That's not an amountSize but the offset of the recipient which is a string.

recipient is a first field of the TokenMessageERC20 structure and since it was encoded with abi.encode there is an offset:

0000000000000000000000000000000000000000000000000000000000000060 # offset of the recipient string = 96, because first three lines are 32 bytes each 0000000000000000000000000000000000000000000000008ac7230489e80000 # amount = 10000000000000000000 0000000000000000000000000000000000000000000000000000000000000000 # chainId = 0 000000000000000000000000000000000000000000000000000000000000002a # recipientSize = 42 3078373143373635364543376162383862303938646566423735314237343031 # "0x71C7656EC7ab88b098defB751B7401B5f6d8976F" 4235663664383937364600000000000000000000000000000000000000000000

Let's leave this comment in the code

src/Pact/Native.hs

ghost · 2024-02-22T10:01:17Z

src/Pact/Native.hs

+      where
+        getWord256be = get @Word256
+
+        -- TODO: We check the size. Is this ok?


This is good 👍

But my comment was about the take and fromIntegral functions since size :: Word256 and we convert it to Int.

src/Pact/Native.hs

* Add hyperlane-decode-token-message with gas * include chainid in TokenMessage encoding * add principal check for recipient * TokenMessage recipient parsed as guard * add new builtin to pact411natives and suppress a debugging error message * exhaust the TokenMessage when parsing * use wide-word instead of doubleword * remove stale TODOs * cleanup more comments * Set the empirically measured cas cost for TokenMessage decoding * fix example in native definition * fix examples * rename builtin to hyperlane-decode-token-message * use base64-unpadded encoding/decoding for tokenmessage * fix gas costing and add gas tests * lower bound for binary dependency * cleanup tokenmessage decoder comments and variable names * remove padding from encoded message in tokenmessage decoding example * gasmodelspec remove buildin from list of gas-untested * update example encoding

imalsogreg added 4 commits February 20, 2024 16:42

Add hyperlane-decode-token-message with gas

9c3f015

include chainid in TokenMessage encoding

0774aca

add principal check for recipient

53494f7

TokenMessage recipient parsed as guard

3d2a171

imalsogreg requested review from emilypi, jmcardon, jwiegley and sirlensalot as code owners February 21, 2024 19:19

imalsogreg added 2 commits February 21, 2024 11:34

add new builtin to pact411natives and suppress a debugging error message

b96ebc6

exhaust the TokenMessage when parsing

bd9fb7b

emilypi reviewed Feb 21, 2024

View reviewed changes

imalsogreg added 6 commits February 21, 2024 13:07

use wide-word instead of doubleword

5d6c156

remove stale TODOs

e2b65c1

cleanup more comments

3631cb8

Set the empirically measured cas cost for TokenMessage decoding

9601df1

fix example in native definition

68a1f51

fix examples

6e7086a

jmcardon reviewed Feb 21, 2024

View reviewed changes

src/Pact/Native.hs Outdated Show resolved Hide resolved

edmundnoble reviewed Feb 21, 2024

View reviewed changes

rename builtin to hyperlane-decode-token-message

89d10ca

ghost reviewed Feb 22, 2024

View reviewed changes

imalsogreg added 5 commits February 22, 2024 09:07

use base64-unpadded encoding/decoding for tokenmessage

f899c60

fix gas costing and add gas tests

2e2703d

lower bound for binary dependency

091c68f

cleanup tokenmessage decoder comments and variable names

3a61688

remove padding from encoded message in tokenmessage decoding example

16f278c

ghost mentioned this pull request Feb 22, 2024

Update hyperlane transaction tests to use hyperlane-decode-token-message. kadena-io/chainweb-node#1833

Merged

gasmodelspec remove buildin from list of gas-untested

4e7f70f

edmundnoble approved these changes Feb 22, 2024

View reviewed changes

src/Pact/Native.hs Outdated Show resolved Hide resolved

update example encoding

bd6d3cb

ghost approved these changes Feb 22, 2024

View reviewed changes

jmcardon approved these changes Feb 22, 2024

View reviewed changes

jmcardon merged commit d5279f4 into master Feb 22, 2024

imalsogreg deleted the greg/hyperlane-decode-message branch February 22, 2024 21:38

Conversation

imalsogreg commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edmundnoble left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

imalsogreg commented Feb 21, 2024 •

edited

Loading