Relayer: Add query parameter support #16

ncomerci · 2025-02-06T20:20:27Z

The purpose of this PR is to implement support and correct handling of parameters for queries. To ensure that the library side functions are correctly typed, both the parameters and their response value, a serialization and deserialization was implemented internally with a JSON library for assemblyscript in order to make the communication with the relayer as simple as possible.

These changes led to change the way inputs.json is generated by reading the .wat file instead of the .ts file.

Still missing:

Adding parameter support to the interfaces that are generated based on ABIs (will be done in a future PR in order not to extend this one)

facuspagnuolo

In general I don't get the purpose of this PR tbh

facuspagnuolo · 2025-02-13T08:13:23Z

packages/relayer/src/oracle.ts

What's the purpose of this class?

Sorry, this is code that I forgot to remove because I was testing. The idea was to separate price-related queries from environment-related queries by classifying them by scope.

facuspagnuolo · 2025-02-13T08:14:26Z

packages/lib-ts/index.ts

I don't get why we have this mapping here between the class and the namespace, what's the reason behind it?

This pr goes together with this PR and is to serialize and deserialize the function parameters. The mapping is for the task to be written using the lib tools and then serialized so that the relayer can read it.

We are thinking with Pedro about a way to make this process more efficient and not having to do it parameter by parameter as it is now.

(Edit) Update: We found a way to use JSON serialization/deserialization for params that improves dev experience

facuspagnuolo · 2025-02-13T08:15:11Z

packages/cli/src/commands/compile.ts

+        })
+    })
+
+  return sortedResult


Why have we dropped the AST logic to produce the inputs file in favor of manual parsing?

Using the task.ts file to generate the inputs.json I found that is necessary to specify in the compile command what it has to look for to generate the inputs. For example: let's suppose that there are 2 contracts defined in the manifest that are used in the task, then the compile, besides looking for the environment functions that were used, it has to identify each contract and for each one, its functions.

On the other hand, if we use the .wat file instead of the task.ts, it is very clear which are the external functions necessary to generate the inputs.json (all of them are identified with an “import” at the beginning of the line). That is why no matter how many external functions are used in the task, the compile does not need to have context and can generate the inputs.json very easily.

packages/cli/src/commands/compile.ts

packages/example/src/task.ts

packages/lib-ts/index.ts

packages/lib-ts/common/index.ts

packages/lib-ts/index.ts

PedroAraoz · 2025-02-18T21:34:37Z

There is a bug with the new generation. I would add some more tests to catch cases like these
If I have the following task

export default function main(): void {
  const usdcAddress = Address.fromString("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48")
  const USDC = ERC20.load(usdcAddress, 1)
  const name = USDC.name()
  const number = USDC.balanceOf(usdcAddress)
  Environment.call(usdcAddress, 1, usdcAddress, usdcAddress, number, Bytes.fromHexString(name))
}

The inputs.json that generates is the following

{
"ERC20": {
  "ERC20": [
    "load"
  ]
},
"index": {
  "environment": [
    "call"
  ]
}
}

ncomerci · 2025-02-19T16:06:32Z

There is a bug with the new generation. I would add some more tests to catch cases like these If I have the following task

export default function main(): void {
  const usdcAddress = Address.fromString("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48")
  const USDC = ERC20.load(usdcAddress, 1)
  const name = USDC.name()
  const number = USDC.balanceOf(usdcAddress)
  Environment.call(usdcAddress, 1, usdcAddress, usdcAddress, number, Bytes.fromHexString(name))
}

The inputs.json that generates is the following

{
"ERC20": {
  "ERC20": [
    "load"
  ]
},
"index": {
  "environment": [
    "call"
  ]
}
}

Hey @PedroAraoz Thank you! I fixed the inputs.json generation to fix this. However, the way the interface is generated will change in the next PR as it doesn't have the correct parameter handling implemented for now.

Edit: Check #20

facuspagnuolo · 2025-02-20T10:23:10Z

packages/cli/tests/fixtures/tasks/task.ts

+  Environment.call(settler, chainId, target, feeToken, fee1, data)
+  Environment.call(settler, chainId, target, feeToken, fee2, data)


Double checking on this, I don't like the idea of calling classes, it's conceptually wrong, this should be an instance of the environment or sth similar to that, that's why this was a namespace initially. Why we are changing this? Is there any other alternative?

Hi Facu! Thanks for the comment. I did a double check on this and, as long as we don't have to handle an internal enviroment state (which is not currently the case), something like this can be done which aligns with what you propose:

Only the functions that are exported are accessible to the user, the others are for internal use.

There were no strong reasons to use a class other than to maintain consistency with the contracts interface which at the moment, having an internal state (address and chain ID), I could not find any other way to implement it other than a class.

Let me know if you agree with this to include it in the PR!

facuspagnuolo · 2025-02-25T15:09:40Z

packages/cli/src/commands/compile.ts

+type FunctionsMap = {
+  [namespace: string]: {
+    [subNamespace: string]: string[]
+  }
+}


For the record, this does not follow our convention to declare types

facuspagnuolo · 2025-02-25T15:11:07Z

packages/cli/src/commands/compile.ts

+
+  const result: FunctionsMap = {}
+
+  const importRegex = /\(import\s+"([^"]+)"\s+"([^"]+)"\s+\(func\s+\$[^\s)]+/


For the record, this does not follow our convention to declare constants

facuspagnuolo · 2025-02-25T15:14:41Z

packages/cli/src/commands/compile.ts

+          sortedInputs[ns][subNs] = inputs[ns][subNs].sort()
+        })
+    })
+  return sortedInputs


For the record, I would love to double-check this is actually better than using the AST

facuspagnuolo · 2025-02-25T15:15:28Z

packages/lib-ts/index.ts

+    feeAmount: BigInt,
+    data: Bytes | null = null
+  ): void {
+    _call(JSON.stringify<CallParams>(new CallParams(settler, chainId, target, feeToken, feeAmount, data)))


For the record, let's analyze the alternative of serializing a list of primitive types instead a single string param (JSON). Not sure if this will be simpler, but it might help to avoid declaring classes for every param structure.

ncomerci added the WIP Work in Progress label Feb 6, 2025

ncomerci self-assigned this Feb 6, 2025

ncomerci changed the base branch from main to lib/intent-related-calls February 12, 2025 19:26

ncomerci requested a review from PedroAraoz February 12, 2025 20:29

ncomerci removed the WIP Work in Progress label Feb 12, 2025

facuspagnuolo reviewed Feb 13, 2025

View reviewed changes

PedroAraoz reviewed Feb 13, 2025

View reviewed changes

ncomerci mentioned this pull request Feb 14, 2025

Lib: Intent related calls #17

Merged

ncomerci requested review from PedroAraoz and facuspagnuolo February 17, 2025 13:32

facuspagnuolo reviewed Feb 18, 2025

View reviewed changes

packages/lib-ts/common/index.ts Outdated Show resolved Hide resolved

packages/lib-ts/index.ts Outdated Show resolved Hide resolved

Base automatically changed from lib/intent-related-calls to main February 18, 2025 20:51

ncomerci added 9 commits February 18, 2025 17:53

feat: base functions case

49a34cd

feat: base async case

2be6506

feat: extract calls from wat file

c6f35ff

fix: test errors

fcc9a20

relayer: remove unused code

f4cd65f

lib: implement param serialization

b3c187b

chore: comments addressed

cee6d59

feat: use optional params

eaef37a

fix: tests

4f7a89a

ncomerci force-pushed the relayer/parameterized-queries-digest branch from ed509ca to 4f7a89a Compare February 18, 2025 21:05

ncomerci added 3 commits February 18, 2025 18:51

fix: rebase

59ce37b

chore: implement requested changes

e4ac191

fix: destination chain can't be null

1db28a8

ncomerci changed the title ~~relayer: Parameterized queries digest~~ Relayer: Add query parameter support Feb 19, 2025

ncomerci added 2 commits February 19, 2025 13:34

fix: template

544555c

fix: rebase error

b635113

facuspagnuolo reviewed Feb 20, 2025

View reviewed changes

chore: replace class for namespace

9617068

facuspagnuolo approved these changes Feb 25, 2025

View reviewed changes

facuspagnuolo merged commit 8b43c4d into main Feb 25, 2025
9 checks passed

facuspagnuolo deleted the relayer/parameterized-queries-digest branch February 25, 2025 13:33

facuspagnuolo reviewed Feb 25, 2025

View reviewed changes

		Environment.call(settler, chainId, target, feeToken, fee1, data)
		Environment.call(settler, chainId, target, feeToken, fee2, data)


		const result: FunctionsMap = {}

		const importRegex = /\(import\s+"([^"]+)"\s+"([^"]+)"\s+\(func\s+\$[^\s)]+/

Relayer: Add query parameter support #16

Relayer: Add query parameter support #16

Uh oh!

Conversation

ncomerci commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Still missing:

Uh oh!

facuspagnuolo 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!

ncomerci Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PedroAraoz commented Feb 18, 2025

Uh oh!

ncomerci commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ncomerci Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ncomerci commented Feb 6, 2025 •

edited

Loading

ncomerci Feb 13, 2025 •

edited

Loading

ncomerci commented Feb 19, 2025 •

edited

Loading

ncomerci Feb 20, 2025 •

edited

Loading