accessing returned contract

[tcurdt]

• [x] I've asked for help in the Truffle Gitter before filing this issue.

---

Issue

The documentation and the API is unclear on how to handle returned contracts.

Steps to Reproduce

Let's say I have the following contracts

contract Contract {

  string private name;

  function setName(string memory newName)
    public
  {
    name = newName;
  }

}

contract Factory {

  function createContract()
    public
    returns (Contract)
  {
    Contract c = new Contract();
    return c;
  }

}

and I want to test this

const Factory = artifacts.require("Factory")

contract("Factory", async accounts => {
  it("creates a contract", async () => {
    let factory = await Factory.new()
    let contract = await factory.createContract.call()
    await contract.setName.call("test")
  })
})

Expected Behavior

I would expect the above test to succeed.

Actual Results

let contract = await factory.createContract() Just returns a transaction.

let contract = await factory.createContract.call() Returns an address - but not a usable object to call on.

let contract = Contract.at(await factory.createContract.call()) Gives Error: Cannot create instance of Contract; no code at address

Environment

• Operating System: macOS 10.13.6
• Ethereum client:
• Truffle version (truffle version): Truffle v5.0.18 (core: 5.0.18)
• node version (node --version): Node v10.15.3
• npm version (npm --version): 6.4.1

[CruzMolina]

Hey @tcurdt , thanks for reporting. Looking into the Solidity docs to verify if this is a bug.

Update: Looks like it is! https://solidity.readthedocs.io/en/v0.5.8/contracts.html#creating-contracts

[tcurdt]

@CruzMolina glad it's not just me :)

Anything else I can do? What should it be like?

[CruzMolina]

Hmm, I was going to suggest a web3.eth.Contract workaround, but it looks like even the current web3 we import as a dependency is unable to handle this scenario.

[tcurdt]

@CruzMolina is it maybe possible to extract the address from the transaction and use that with Contract.at(..)? just as a work around? I am a little stuck at the moment.

[tcurdt]

This works but feels like a hack:

const Factory = artifacts.require("Factory")

contract("Factory", async accounts => {
  it("creates a contract", async () => {
    const factory = await Factory.new()
    const tx = await factory.createContract()
    const address = tx.logs[0].address
    const center = await Center.at(address)
    await contract.setName("test")
  })
})

[CruzMolina]

Hmm, interesting. I seem to be unable to get any logs from the createContract call locally (while using truffle develop or truffle test).

Can you share an example repo or reproduction steps?

[CruzMolina]

BTW, although your working code seems hacky, that looks like the correct way to accomplish what you're trying to do.

[tcurdt]

Odd. I am using it with npx truffle test and v5.0.18. And there is not much more to it. I cannot share the full repo but creating a test case should be doable.

[CruzMolina]

Are you having truffle test connect to another chain other than the default network that automatically spins up?

[CruzMolina]

I'm finding that tx.logs just returns an empty array on my end.

[tcurdt]

Nope. Built-in ganache. Nothing in truffle.js but the compiler settings. Odd.

[CruzMolina]

Is your contract source code the same as when you opened the issue?

[CruzMolina]

What are your compiler settings, btw?

[tcurdt]

Attached is a test case. Seems like the tx.logs is coming from the emit that was missing from the issue here.

What is strange with the test case, the setName causes a revert. Unfortunately there is no clear indication why.

testcase.zip

[CruzMolina]

@tcurdt, I believe your event is emitting the address of the Factory contract instead of the newly created contract.

Try updating your emit line to be:

    emit LogFooAdd(msg.sender, now, address(c));

and your test

contract("Factory", async accounts => {
  it("creates foo", async () => {
    const factory = await Factory.new()
    const { logs } = await factory.createFoo()
    const address = logs[0].args.instance
    const foo = await Foo.at(address)
    await console.log(address)
    await foo.setName("test")
  })
})

🤞

[tcurdt]

Thanks for the help.. Now I slowly understand where the tx.logs comes from. This feels even more like hack now :)

The event is related to the factory so it really was supposed to be this to give the context of the factory. But I could of course add the contract address as well.

That said - in the original code base passes just this. Which is why I am wondering where the proper address is coming from. The tests are passing using tx.logs[0].address. Guess I need to do some more digging.

Anyway - but that's just for the work around. There should be proper support for this, no?

[CruzMolina]

I believe tx.logs[0].address is the address that tx is being sent to, which is the Factory contract.

You should be able to verify that tx.logs[0].address and tx.logs[0].args.instance are the same address given the way you have the event currently set up (msg.sender and the Factory address in this).

You're right that const foo = await Foo.at(this) should fail. Currently it doesn't look like there's a check until you actually try to call or transact with foo.

[tcurdt]

In the real code base I am calling/transacting with what here is foo. Hence my surprise it's actually working. I will try to dig in more.

But for the future it should become just foo = await factory.createFoo(), right?

[CruzMolina]

It's hard to say definitively what should or shouldn't be happening without a look into the code.

IIRC, in production const foo = await factory.createFoo() w/o an event that emits the newly created contract address should not return anything more a normal tx log (no newly created contract address in the logs).

[tcurdt]

What code to look into do you mean?

IMO it's very odd to have the function specified to return a contract (or address) but on the javascript side all you get is a transaction. And then having to "ship" the return value via the event system. And in other cases calling contract function happily return values.

[CruzMolina]

What code to look into do you mean?

Without a public repo it's hard to reproduce and help with debugging.

IMO it's very odd...

Def agreed. I could be mistaken but it appears that behavior is up-to-spec with the way the EVM currently works in practice.

[gnidan]

Is this work to be done for this issue to make Truffle work better? Can we identify a definition of done so we can close? It sounds like there might be room for improvements to our docs.

Unfortunately I don't think we can change the return values for a transaction, the way you are looking for, but we might be able to improve the event observation syntax to make that easier. Let us know what you think! Thanks!

[tcurdt]

@gnidan I understand that backwards compatibility is a problem. As for options I am not sure I have enough in-depth knowledge.

A work around I use now in my code base is

exports.factory = async function(o, tx) {
  const address = tx.logs[0].address
  const contract = await o.at(address)
  return contract
}

and then

const foo = await factory(Foo, await api.createFoo())

which is all but ideal - but it's a start.

Improving the docs would be much appreciated for sure. Maybe something along the lines of what @CruzMolina suggested could find its way to the docs.

But in general it's just very hard to explain that contract.getName() returns a value but contract.createFoo() returns a transaction - at least from the API perspective.

Even worse (and I am just assuming here and didn't test it but it that's how I understood things) is that if contract.getName() starts to emit an event it will change to return a transaction instead of a value. If that's really the case - oh the horror.

[gnidan]

The open question, in my mind, is: what's the best syntactic sugar _{(ref: Wikipedia)} for getting information out of a transaction? I view this "how do we get a factory-made contract instance?" as a subset of that problem... I'm happy to lend my views toward factory-specific syntax, but I think that's a later discussion.

My concerns are that:

• The JSON RPC _{(ref: Ethereum Wiki)} prevents getting return values from transactions. IIRC, this is by design, although I don't recall why exactly... I imagine it might be for implementation reasons (nodes would have to store return values for every transaction they validate, to enable later lookup, and how do you price the gas?).
• Transactions do "return" logs, however, which we can interpret as Solidity's events. But! Each transaction can emit n of these, with arbitrary arguments and arbitrary names. Which event do we pick? Which event means "the return value"?

Considering those, I think doing this in an unhacky way poses some challenges:

• Unhacky syntax forces users to specify event filters themselves, likely by name, but that might not be good enough... name, index? name, specific argument values? This contract abstraction can't just assume logs[0].

• How can this syntax clearly link between the transaction and the event? It's annoying and confusing to do something like:

  let result;
  
  // listen first - note we can't wrap this in a promise to `await`
  factory.Created.on("data", ({ /* ... */ }) => {
  result = { /* ... */ };
  });
  
  // transaction second
  await factory.createFoo();

  ^{code snippet i.: Listener then transaction}

  It's also annoying to do:

  // transaction first, capturing result
  const { receipt: { logs } } = await factory.createFoo();
  
  // digging second
  const result = logs
  .filter(/* ... to find which event ... */)
  .map(/* ... to get the specific value from the event (e.g. address, or `Foo` instance) */)
  [0]; /* ugh gross */

  ^{code snippet ii.: Transaction then inspect log}

• Can we get away with doing some light magic—i.e. provide means to get the a single log from the last transaction implicitly?

  Something like this might look like the start of an approach:

  // run transaction simply
  await factory.createFoo();
  
  // magically grab the info via `.EventName()` syntax...
  // implicitly grab the first? the last?
  const { args: { address } } = await factory.Created();
  
  // get the contract
  const foo = await Foo.at(address);

  ^{code snippet iii.: Magic hidden transaction record}

  Note: I am generally opposed to this approach, but wanted to include it because I think there's an argument for it.

• Maybe we can add some kind of Events interface to the receipt:

  const { tx, receipt: { events } } = await factory.createFoo()
  
  // just map to handle plural condition (in tests, one might want to ensure uniqueness)
  const newFoos = events.Created()
  .map( async ({ args: { address } }) => await Foo.at(address) );
  assert.lengthOf(newFoos, 1, "There should be only one");
  const foo = newFoos[0];
  
  // or maybe we get wild
  const foo = events.Created()
  .map( async ({ args: { address } }) => await Foo.at(address) )
  .first();  // other name ideas? `.only()` `.event()` `[0]`

  ^{code snippet iv.: Receipt Events interface}

  This sort of thing is what I'm leaning towards right now, but definitely would require further consideration.

Hopefully this helps you see my perspective on this issue. I recognize that it's rather nasty to do what you want, as things stand today, but it does seem promising that there may be a path forward.

Anyway, I'll wrap this comment up with some reassurance:

Even worse (and I am just assuming here and didn't test it but it that's how I understood things) is that if contract.getName() starts to emit an event it will change to return a transaction instead of a value. If that's really the case - oh the horror.

Calls will never emit events! Calls are read-only; logging is considered a write. So we're safe there :)

Thanks again for prompting this discussion!

[tcurdt]

Wow - a great response. Let me start with with where I am struggling first. You make the distinction between calls and transactions. But my point was that this distinction is not clear from the js API point of view.

contract Foo {

  string private name;

  function getName()
    public view
    returns (string memory)
  {
    return name;
  }
}

On the solidity side it's clear this is a view which makes this a read-only call. So is the usage on the js side. Let's say (awkward use case but for the sake of argument) we want to log access to getName(). If view gets removed and an event gets emitted this becomes a transaction.

contract Foo {

  event LogAccess(address indexed owner, uint indexed accessTime);

  string private name;

  function getName()
    public
    returns (string memory)
  {
    emit LogAccess(msg.sender, now);
    return name;
  }
}

IIUC the usage would still be the same in solidity but on the js side suddenly there is no easy way to get to the returned value. Just want to make sure I am not wrong and we are on the same side before talking on how to improve this.

[eggplantzzz]

This seems like a very complex issue that will require some further discussion. We are going to review this later, feel free to ping us if you feel like this is an urgent issue for you. Thanks y'all!

[tcurdt]

Not sure it's really complex as such but it surely questions how the solidity api is currently translated to js. I got my work around but this seems like a very fundamental thing worth solving. Let me know if this needs further input from my side.

[vicnaum]

Btw, in Remix we also get a transaction, but it has: decoded output | { "0": "address: 0x194146544073844DFdA12790823243f954F1A2D2" }

[eliotstock]

Two years later and I still don't see how I'm supposed to get a usable contract instance in my JS test here.

Why does this line:

https://github.com/eliotstock/eth-mortgage/blob/main/test/loanpool.js#L43

fail with:

Error: Returned error: VM Exception while processing transaction: revert

Did you update the docs? I don't see anything there.