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

feat: add ARC22 and ARC28 interfaces for ABI contracts and methods #856

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

joe-p
Copy link
Contributor

@joe-p joe-p commented Feb 17, 2024

Adds ARC28 (events) and ARC22 (readonly) to ABI-related interfaces

@joe-p joe-p changed the title feat: add ARC22 readonly to ABIMethodParams feat: add ARC22 and ARC28 interfaces for ABI contracts and methods Apr 11, 2024
@joe-p joe-p requested a review from a team as a code owner April 11, 2024 15:40
@joe-p joe-p requested a review from onetechnical April 11, 2024 15:40
@joe-p

This comment was marked as outdated.

@pbennett
Copy link

pbennett commented May 2, 2024

Just chiming in to say this would be nice to get in - it's preventing me from cleanly including ARC28 events in my tealscript contracts. The generated json blocks tests from being run because it doesn't match the schema defined in the sdk.

@onetechnical onetechnical requested review from jasonpaulos and removed request for onetechnical May 8, 2024 14:31
src/abi/contract.ts Outdated Show resolved Hide resolved
src/abi/method.ts Show resolved Hide resolved
src/abi/event.ts Show resolved Hide resolved
src/abi/interface.ts Outdated Show resolved Hide resolved
@@ -77,6 +82,7 @@ export class ABIMethod {
}>;

public readonly returns: { type: ABIReturnType; description?: string };
public readonly events: ARC28Event[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're still missing readonly

@jasonpaulos
Copy link
Member

jasonpaulos commented May 31, 2024

Also it looks like this failing in cucumber tests:

13) Scenario: Serialize Contract into json # tests/cucumber/features/unit/abijson.feature:97
    ✔ When I create the Method object from method signature "add(uint32,uint32)uint32" # tests/cucumber/steps/index.js:510
    ✔ And I create a Contract object from the Method object with name "ExampleContract" and description "This is an example contract" # tests/cucumber/steps/index.js:510
    ✔ And I set the Contract's appID to 1234 for the network "wGHE2Pwdvd7S12BL5FaOP20EGYesN73ktiC1qzkkit8=" # tests/cucumber/steps/index.js:510
    ✔ And I set the Contract's appID to 5678 for the network "SGO1GKSzyE7IEPItTxCByw9x8FmnrCDexi9/cOUJOiI=" # tests/cucumber/steps/index.js:510
    ✔ And I serialize the Contract object into json # tests/cucumber/steps/index.js:510
    ✖ Then the produced json should equal "contract.json" loaded from "abi_responsejsons" # tests/cucumber/steps/index.js:579
        AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
        + actual - expected ... Lines skipped

          {
            desc: 'This is an example contract',
        +   events: [],
        +   methods: [
        +     {
        +       args: [
        +         {
        +           type: 'uint32'
        +         },
        +         {
        +           type: 'uint32'
        +         }
        +       ],
        +       events: [],
        -   methods: [
        -     {
        -       args: [
        -         {
        -           type: 'uint32'
        -         },
        -         {
        -           type: 'uint32'
        -         }
        -       ],
                name: 'add',
        ...
              }
            }
          }

Perhaps when converting to JSON, if no events are present, that field should be omitted from the result? Since these tests run on every SDK, they're a bit difficult to deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants