All system contract compatibility and better tests #52

aaroncox · 2023-08-20T17:38:10Z

Alright… some updates!

I fixed the dot notation issue I mentioned with the eosio contract, which led to a bunch of other things - some of which I also managed to fix. But there’s more 😅

The dot notation thing was that in the system contract, there are tables with . in the name, which when passed in to the factory, was creating object properties with . in them, like powup.state: value and causing errors.

Fix was easy, using JSON.stringify(): f6cfcaf

I then went in and rewrote the tests to loop over and test against 4 contracts: eosio , eosio.token, eosio.msig and rewards.gm. All the testing data is setup to process these, but the tests are commented out preventing the failure. To enable them, uncomment here:

contract/test/tests/codegen.ts

Lines 7 to 10 in 6de4f4c

    
           // import Eosio from '$test/data/contracts/mock-eosio' 
        
           import EosioToken from '$test/data/contracts/mock-eosio.token' 
        
           import RewardsGm from '$test/data/contracts/mock-rewards.gm' 
        
           // import EosioMsig from '$test/data/contracts/mock-eosio.msig'

and here:

contract/test/tests/codegen.ts

Lines 25 to 32 in 6de4f4c

    
           // eosio: { 
        
           //     mock: Eosio, 
        
           //     generated: null, 
        
           // }, 
        
           // 'eosio.msig': { 
        
           //     mock: EosioMsig, 
        
           //     generated: null, 
        
           // },

This is going to cause a number of errors that need to be fixed. There's a bunch of edge cases I don't think we noticed in our limited testing that these additional contracts are now showing. This includes:

abi.types

The "types" top level field of the ABI needs to be used to resolve mapping issues. These are "mappings" essentially between various types and the eosio contract uses them. At build time we need to read this data and link things properly. Here's the relevant part from the eosio contract abi and an example:

{
    "version": "eosio::abi/1.2",
    "types": [
        {
            "new_type_name": "block_signing_authority",
            "type": "variant_block_signing_authority_v0"
        }
    ],
    "structs": [
        {
            "name": "block_signing_authority_v0",
            "base": "",
            "fields": [
                {
                    "name": "threshold",
                    "type": "uint32"
                },
                {
                    "name": "keys",
                    "type": "key_weight[]"
                }
            ]
        },
        {
            "name": "regproducer2",
            "base": "",
            "fields": [
                {
                    "name": "producer",
                    "type": "name"
                },
                {
                    "name": "producer_authority",
                    "type": "block_signing_authority"
                },
                {
                    "name": "url",
                    "type": "string"
                },
                {
                    "name": "location",
                    "type": "uint16"
                }
            ]
        },
    ],
    "variants": [
        {
            "name": "variant_block_signing_authority_v0",
            "types": [
                "block_signing_authority_v0"
            ]
        }
    ]
}

You can see regproducer2 using block_signing_authority. This isn't a struct, it's a top level type, that maps to a variant. Resolving this will lead to the appropriate type...

block_signing_authority (regproducer2.producer_authority)
-> variant_block_signing_authority_v0 (from types on the abi)
    -> block_signing_authority_v0 (from variants on the abi)

This would mean that on the regproducer2 action, we should map it like this:

contract/test/data/contracts/mock-eosio.ts

Lines 1117 to 1127 in 6de4f4c

    
           @Struct.type('regproducer2') 
        
           export class Regproducer2 extends Struct { 
        
               @Struct.field(Name) 
        
               producer!: Name 
        
               @Struct.field(BlockSigningAuthorityV0) 
        
               producer_authority!: BlockSigningAuthorityV0 
        
               @Struct.field('string') 
        
               url!: string 
        
               @Struct.field(UInt16) 
        
               location!: UInt16 
        
           }

On the above link, notice how I already corrected the "mock" version of the code to be what's expected 👍

hoisting

Within the type declarations of the generated code we're going to need to somehow do hoisting.

If you generate the eosio contract, you'll notice that going in order from the ABI the generated code will throw errors. It's because some types are being used before they are defined, like with ProducerKey and ProducerSchedule. We need the output code to detect this and then order them so these errors don't occur.

The abi2core library did this, and has a function which might solve this problem:

https://github.com/greymass/abi2core/blob/master/src/toposort.ts

missing types

Some types aren't resolving properly, you'll notice these if you run against the eosio and eosio.msig contracts. Things like block_timestamp_type or Varuint32. In the "mock" versions of the contracts I fixed these, but the generator needs to have a mapping for these.

If you simply run the tests you'll see them!

values ending with `$`

Unsure what's going on, but there are lots of $ at the end some values that aren't being removed. You'll see these again if you run the tests.

Fixes #53

It was casting undefined to a string and never using `this.account`

This matches the syntax used in the CDT, as discussed here: https://github.com/orgs/wharfkit/discussions/5#discussioncomment-6515701

dafuga · 2023-08-22T23:51:39Z

src/codegen/helpers.ts

-    'UInt64',
-    'UInt8',
-]
+const EOSIO_CORE_CLASSES: string[] = []


We should probably rename this variable too 😄 Maybe to ANTELOPE_CLASSES?

Totally agreed, didn't even think about that 😂

dafuga · 2023-08-22T23:55:36Z

src/codegen/interfaces.ts

@@ -68,3 +69,21 @@ export function generateActionsNamespace(abi: ABI.Def): ts.ModuleDeclaration {
        ts.NodeFlags.Namespace
    )
 }
+
+function findParamTypeString(typeString: string, namespace: string | null, abi: ABI.Def): string {


Is this method specific to interfaces? If not, I would put it in the helper.ts file.

I think it is exclusive to the interfaces, since it's only used for ActionParams as it finds the loose types like NameType or IntType.

dafuga · 2023-08-22T23:56:02Z

src/codegen/maps.ts

 import * as ts from 'typescript'
 import {findAbiType} from './helpers'

 export function generateTableMap(abi: ABI.Def): ts.VariableStatement {
    // Map over tables to create the object properties
    const tableProperties = abi.tables.map((table) =>
        ts.factory.createPropertyAssignment(
-            String(table.name),
+            JSON.stringify(table.name),


Right? lol....

dafuga · 2023-08-22T23:58:38Z

src/codegen/structs.ts

+        return 'boolean'
+    }
+
+    if (['String', 'Boolean', 'Number'].includes(fieldType)) {


Nice catch, but if Boolean isn't a thing then we should probably remove it from this line.

I'm not sure if it is or isn't a thing, so I didn't want to remove it 😅

dafuga · 2023-08-23T00:30:33Z

test/tests/generic.ts

@@ -0,0 +1,77 @@
+import {Action, Name} from '@wharfkit/antelope'


I wonder if we need a tests/helpers folder for functions like these 🤔

dafuga · 2023-08-23T01:08:34Z

test/tests/table.ts

+            })
+            const contract = await contractKit.load('eosio.token')
+            const result = await contract.table('accounts', 'wharfkittest').get()
+            assert.instanceOf(result.balance, Asset)


Does this test really make sure that the correct scope is used? 🤔 Wouldn't the balance field always return an Asset?

dafuga

I left you some comments, but this looks great! I think that we can go ahead and merge this whenever you're ready

aaroncox · 2023-08-24T22:46:52Z

@dafuga I didn't want to merge this yet. Instead I figured I'd just hand this branch off to you to work with, since the original comment on this PR still has a bunch of code generation stuff that needs to be addressed to fix the failing tests within.

dafuga · 2023-08-25T01:56:36Z

Sounds good! I'll make a new PR using this as a base branch and we can just merge them both at the same time.

Codegen touchups

aaroncox added 15 commits August 20, 2023 10:37

Rewrote tests to remove recursion and throw

909fc68

removed try/catch

7ea64c6

Fixed issue with Symbol typing

3441ddd

Switching imports to wharfkit/antelope

4d23ec2

catching Bool type

2f749a3

Dynamically import all core types related to ABIs

45059e9

Adding missing types

c9aa1a0

Fixing TableNames key stringify

f6cfcaf

Updating tests to support many contracts

9a1a062

Changing imports over to wharfkit/antelope instead of session

6de4f4c

Removed notes

dce942a

Only override action and table call if data for those exist

561a515

Scopes should accept generic numbers and cast to strings

1d1b64f

Fixes #53

Fixed scope logic

69a3295

It was casting undefined to a string and never using `this.account`

Allow passing scope as 2nd param to contract.table

3b2e81c

This matches the syntax used in the CDT, as discussed here: https://github.com/orgs/wharfkit/discussions/5#discussioncomment-6515701

dafuga reviewed Aug 22, 2023

View reviewed changes

dafuga reviewed Aug 23, 2023

View reviewed changes

dafuga approved these changes Aug 23, 2023

View reviewed changes

Version 0.4.1

aa0af60

dafuga added 5 commits August 24, 2023 20:52

chore: renamed EOSIO_CORE variables

7c0c642

refactor: added helper folder

382a8d3

enhancement: making sure that Name is always in contract imports

ec25a8b

fix: removing $ from field name when applicable

2eff3da

chore: added full eosio abi

127e602

dafuga and others added 9 commits August 25, 2023 12:04

cleanup: removed contracts folder

fcfadeb

fix: parsing data types properly

12b04b8

fix: using proper eosio abi

1b42c88

fix: handling variant and alias types

09aa3f1

fix: handling booleans properly

e4f1f3e

fix: handling variants in external and internal types

eb802a7

cleanup: removed redundant Types namespace from Struct field type

97675bc

style: linted

e1fe51f

Merge pull request #54 from wharfkit/codegen-touchups

5d0ca65

Codegen touchups

aaroncox changed the title ~~Rewrote tests to remove recursion and throw~~ All system contract compatibility and better tests Aug 26, 2023

aaroncox merged commit d1b2afd into dev Aug 26, 2023
3 checks passed

dafuga deleted the better-tests branch August 28, 2023 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

All system contract compatibility and better tests #52

All system contract compatibility and better tests #52

aaroncox commented Aug 20, 2023 •

edited

Loading

dafuga Aug 22, 2023

aaroncox Aug 23, 2023

dafuga Aug 22, 2023

aaroncox Aug 23, 2023

dafuga Aug 22, 2023

aaroncox Aug 23, 2023

dafuga Aug 22, 2023 •

edited

Loading

aaroncox Aug 23, 2023

dafuga Aug 23, 2023

dafuga Aug 23, 2023 •

edited

Loading

dafuga left a comment •

edited

Loading

aaroncox commented Aug 24, 2023

dafuga commented Aug 25, 2023

	// import Eosio from '$test/data/contracts/mock-eosio'
	import EosioToken from '$test/data/contracts/mock-eosio.token'
	import RewardsGm from '$test/data/contracts/mock-rewards.gm'
	// import EosioMsig from '$test/data/contracts/mock-eosio.msig'

	// eosio: {
	// mock: Eosio,
	// generated: null,
	// },
	// 'eosio.msig': {
	// mock: EosioMsig,
	// generated: null,
	// },

	@Struct.type('regproducer2')
	export class Regproducer2 extends Struct {
	@Struct.field(Name)
	producer!: Name
	@Struct.field(BlockSigningAuthorityV0)
	producer_authority!: BlockSigningAuthorityV0
	@Struct.field('string')
	url!: string
	@Struct.field(UInt16)
	location!: UInt16
	}

		@@ -0,0 +1,77 @@
		import {Action, Name} from '@wharfkit/antelope'

All system contract compatibility and better tests #52

All system contract compatibility and better tests #52

Conversation

aaroncox commented Aug 20, 2023 • edited Loading

abi.types

hoisting

missing types

values ending with $

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dafuga Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dafuga Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

dafuga left a comment • edited Loading

Choose a reason for hiding this comment

aaroncox commented Aug 24, 2023

dafuga commented Aug 25, 2023

aaroncox commented Aug 20, 2023 •

edited

Loading

values ending with `$`

dafuga Aug 22, 2023 •

edited

Loading

dafuga Aug 23, 2023 •

edited

Loading

dafuga left a comment •

edited

Loading