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

[Test] Preview release - Constructing CompositeType using TestAccount fails #229

Open
sisyphusSmiling opened this issue Oct 25, 2023 · 1 comment
Labels
Bug The issue represents a bug, malfunction, regression Feedback

Comments

@sisyphusSmiling
Copy link

Current Behavior

Attempting to construct a CompositeType from an identifier including the address of a test account results in an error. This can be seen in test runs in ci runs for flow-ft and flow-nft Cadence 1.0 updates where test cases cover events emitted by attempting to construct a CompositeType of the event.

The resulting error outputs as:

- FAIL: testContractInitializedEventEmitted
		Execution failed:
			error: internal error: could not find account with address 0000000000000005

Where the failing test case looks like:

access(all) fun testContractInitializedEventEmitted() {
    let typ = CompositeType(buildTypeIdentifier(admin, "ExampleNFT", "ContractInitialized"))!

    Test.assertEqual(1, blockchain.eventsOfType(typ).length)
}

// helper
access(all) fun buildTypeIdentifier(_ acct: Test.TestAccount, _ contractName: String, _ suffix: String): String {
    let addrString = acct.address.toString()
    return "A.".concat(addrString.slice(from: 2, upTo: addrString.length)).concat(".").concat(contractName).concat(".").concat(suffix)
}

Expected Behavior

I would expect the CompositeType to construct given a properly formatted identifier containing a valid contract address, name, and event type.

Steps To Reproduce

Either of the PRs in flow-ft & flow-nft have failing test cases that showcase this error, but the following simple case might help for easy illustration:

Say you have a project with the contract below:

// ./contracts/Foo.cdc
access(all) contract Foo {

    access(all) event ContractInitialized(blockHeight: UInt64)

    init() {
        emit ContractInitialized(blockHeight: getCurrentBlock().height)
    }
}

The following scripts:

// ./scripts/return_account_address.cdc
access(all) fun main(address: Address): Address {
    return getAccount(address).address
}

// ./scripts/has_foo_deployed.cdc
access(all) fun main(address: Address): Bool {
    return getAccount(address).contracts.get(name: "Foo") != nil
}

And the following test:

// ./tests/test_foo.cdc
import Test

access(all) let blockchain = Test.newEmulatorBlockchain()

access(all) let foo = blockchain.createAccount()

access(all) fun setup() {

    blockchain.useConfiguration(Test.Configuration(
            addresses: {
                "Foo": foo.address
            }
    ))

    let err = blockchain.deployContract(
        name: "Foo",
        code: Test.readFile("../contracts/Foo.cdc"),
        account: foo,
        arguments: [],
    )

    Test.expect(err, Test.beNil())
    if err != nil {
        panic(err!.message)
    }

}

access(all) fun testContractInitialized() {
    // Fails when attempting to construct this type
    let typ = CompositeType(buildTypeIdentifier(foo, "Foo", "ContractInitialized"))!

    Test.assertEqual(1, blockchain.eventsOfType(typ).length)
}

access(all) fun testContractInitializedAlt() {
    // Fails even when hardcoding address string
    let typ = CompositeType("A.0000000000000005.Foo.ContractInitialized")!

    Test.assertEqual(1, blockchain.eventsOfType(typ).length)
}

// Confirms the account is accessible in the test environment
access(all) fun testGetAccountAddress() {
    let code = Test.readFile("../scripts/return_account_address.cdc")
    let scriptResult = blockchain.executeScript(code, [foo.address])

    if let failureError = scriptResult.error {
        panic("Failed to execute the script because -:  ".concat(failureError.message))
    }

    let address = scriptResult.returnValue as! Address? ?? panic("Could not retrieve address")
    Test.assertEqual(foo.address, address)
}

// Confirms Foo is deployed to the account in question
access(all) fun testHasFooDeployed() {
    let code = Test.readFile("../scripts/has_foo_deployed.cdc")
    let scriptResult = blockchain.executeScript(code, [foo.address])

    if let failureError = scriptResult.error {
        panic("Failed to execute the script because -:  ".concat(failureError.message))
    }

    let hasFoo = scriptResult.returnValue as! Bool? ?? panic("Could not retrieve contract names from foo")
    Test.assertEqual(true, hasFoo)
}

access(all) fun buildTypeIdentifier(_ acct: Test.TestAccount, _ contractName: String, _ suffix: String): String {
    let addrString = acct.address.toString()
    return "A.".concat(addrString.slice(from: 2, upTo: addrString.length)).concat(".").concat(contractName).concat(".").concat(suffix)
}

Running the following command:

flow test --cover tests/test_foo.cdc

Shortened results shown below:

Test results: "tests/test_foo.cdc"
- FAIL: testContractInitialized
		Execution failed:
			error: internal error: could not find account with address 0000000000000005
- FAIL: testContractInitializedAlt
		Execution failed:
			error: internal error: could not find account with address 0000000000000005
- PASS: testGetAccountAddress
- PASS: testHasFooDeployed

Environment

- Cadence version: v1.5.0-stable-cadence.3
- Network: Emulator
@sisyphusSmiling sisyphusSmiling added Bug The issue represents a bug, malfunction, regression Feedback labels Oct 25, 2023
@SupunS
Copy link
Member

SupunS commented Oct 30, 2023

I think what's happening here is the created account foo is on the blockchain, whereas the composite type creation CompositeType("A.0000000000000005.Foo.ContractInitialized") happens off-chain (in the test script). So the account is not accessible/available off-chain. This is not really a bug rather the expected behavior.

I believe the new changes done to unify the environment (and removing the blockchain concept) and the improvement done in this PR should allow this use-case now. Given the changes are already on master, they would be available with the next CLI preview build.
cc: @m-Peter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug The issue represents a bug, malfunction, regression Feedback
Projects
None yet
Development

No branches or pull requests

2 participants