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

Fix missing nested dependencies in fuel types gen causing incorrect ordering of code generation #257

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

DenhamPreen
Copy link
Contributor

@DenhamPreen DenhamPreen commented Oct 9, 2024

The following type from the spark abi was resulting in the

 {
      "type": "(_, _, _, _, _, _, _)",
      "metadataTypeId": 4,
      "components": [
        {
          "name": "__tuple_element",
          "typeId": 40
        },
        {
          "name": "__tuple_element",
          "typeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc"
        },
        {
          "name": "__tuple_element",
          "typeId": 40
        },
        {
          "name": "__tuple_element",
          "typeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc"
        },
        {
          "name": "__tuple_element",
          "typeId": 19,
          "typeArguments": [
            {
              "name": "",
              "typeId": 18
            }
          ]
        },
        {
          "name": "__tuple_element",
          "typeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc"
        },
        {
          "name": "__tuple_element",
          "typeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc"
        }
      ]
    },

Was giving a dependencies of
["type40", "type47", "type40", "type47", "type19", "type47", "type47"] - missing the type18

This was causing the codegen of the rescript schema to generate in the wrong order where the above type4Schema was generated before the type18Schema yet it was referencing the type18Schema in type4Schema.

The solution was to account for nested dependency types.

@DenhamPreen DenhamPreen force-pushed the dp/fix-fuel-gen-sort-algorithm branch from 8b75d5e to 615a0d6 Compare October 9, 2024 16:42
Copy link
Contributor

@JasoonS JasoonS 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, will test properly first thing in the morning

@JasoonS
Copy link
Contributor

JasoonS commented Oct 10, 2024

This is the diff on the Spark codebase. (Just changes the order in which the types are declared)

Looks good. It's hard for me to know if this could possibly cause other unexpected issues with other indexes, but from what I can see and from what I know I'm happy.

diff --git a/src/Types.res b/src/Types.res
index b525f1a..580943a 100644
--- a/src/Types.res
+++ b/src/Types.res
@@ -853,7 +853,6 @@ let type39Schema = S.object((s): type39 => {bits: s.field("bits", type45Schema)}
 let type40Schema = S.object((s): type40 => {bits: s.field("bits", type45Schema)})
 let type41Schema = S.object((s): type41 => {bits: s.field("bits", type45Schema)})
 let type42Schema = (_tSchema: S.t<'t>) => S.object((s): type42<'t> => {ptr: s.field("ptr", type22Schema), cap: s.field("cap", type48Schema)})
-let type4Schema = S.tuple(s => (s.item(0, type40Schema), s.item(1, type47Schema), s.item(2, type40Schema), s.item(3, type47Schema), s.item(4, type19Schema(type18Schema)), s.item(5, type47Schema), s.item(6, type47Schema)))
 let type18Schema = S.union([S.object((s): type18 =>
 {
   s.tag("case", "Address")
@@ -873,6 +872,7 @@ let type31Schema = S.object((s): type31 => {amount: s.field("amount", type48Sche
 let type36Schema = S.object((s): type36 => {base_sell_order_id: s.field("base_sell_order_id", type45Schema), base_buy_order_id: s.field("base_buy_order_id", type45Schema), base_sell_order_limit: s.field("base_sell_order_limit", type6Schema), base_buy_order_limit: s.field("base_buy_order_limit", type6Schema), order_matcher: s.field("order_matcher", type18Schema), trade_size: s.field("trade_size", type48Schema), trade_price: s.field("trade_price", type48Schema), block_height: s.field("block_height", type47Schema), tx_id: s.field("tx_id", type45Schema), order_seller: s.field("order_seller", type18Schema), order_buyer: s.field("order_buyer", type18Schema), s_balance: s.field("s_balance", type23Schema), b_balance: s.field("b_balance", type23Schema), seller_is_maker: s.field("seller_is_maker", type46Schema)})
 let type37Schema = S.object((s): type37 => {amount: s.field("amount", type48Schema), asset: s.field("asset", type40Schema), user: s.field("user", type18Schema), account: s.field("account", type23Schema)})
 let type38Schema = S.object((s): type38 => {amount: s.field("amount", type48Schema), asset: s.field("asset", type40Schema), user: s.field("user", type18Schema), account: s.field("account", type23Schema), market: s.field("market", type41Schema)})
+let type4Schema = S.tuple(s => (s.item(0, type40Schema), s.item(1, type47Schema), s.item(2, type40Schema), s.item(3, type47Schema), s.item(4, type19Schema(type18Schema)), s.item(5, type47Schema), s.item(6, type47Schema)))
 let type17Schema = S.union([S.object((s): type17 =>
 {
   s.tag("case", "Uninitialized")

@JasoonS JasoonS merged commit 15eaa59 into main Oct 10, 2024
1 check passed
@JasoonS JasoonS deleted the dp/fix-fuel-gen-sort-algorithm branch October 10, 2024 06:00
@JonoPrest
Copy link
Collaborator

Really nice catch and fix @DenhamPreen 👏🏼

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.

3 participants