web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.31k stars 4.95k forks source link

Wrong addresses when instancing two contract in contructor #2502

Closed ilanolkies closed 5 years ago

ilanolkies commented 5 years ago

Description

I'm creating two web3.eth.Contract objects in a class constructor. Of course, both contract's addresses are different. After creating the objects the addresses of the Contract are the same. They both have the second contract address.

Expected behavior

Calling two consectuive times new web3.eth.Contract should instance two contracts correctly.

Actual behavior

constructor (web3, registrarAddress, rifAddress, options = {}) {
    console.log('registrar before: ' + registrarAddress)
    console.log('rif before: ' + rifAddress)
    this.contract = new web3.eth.Contract(registrarAbi, registrarAddress, options);
    this.rif = new web3.eth.Contract(rifAbi, rifAddress, options);
    console.log('registrar after: ' + this.contract.address)
    console.log('rif after: ' + this.rif.address)
}

Logs:

Registrar.js:22 registrar before: 0x95bfefa36f4c0504560da00c0d04fc30ef45d4c6
Registrar.js:23 rif before: 0x38632564f438967e8e3259bac2f3877bb4b5104f
Registrar.js:26 registrar after: 0x38632564f438967e8e3259bac2f3877bb4b5104f
Registrar.js:27 rif after: 0x38632564f438967e8e3259bac2f3877bb4b5104f

Steps to reproduce the behavior

I have some inner classes:

index.js

export default class RNS {
  constructor (web3Provider, options = {}, addresses = defaultAddresses) {
    const web3 = new Web3(web3Provider, options);
    this.registrar = new Registrar(web3, addresses.registrarAddress, addresses.rifAddress, options);
  }
}

Registrar.js

export default class Registrar {
  constructor (web3, registrarAddress, rifAddress, options = {}) {
    this.contract = new web3.eth.Contract(registrarAbi, registrarAddress, options);
    this.rif = new web3.eth.Contract(rifAbi, rifAddress, options);
  }
}

Error Logs

Already provided

Versions

ilanolkies commented 5 years ago

What do you need to clariffy?

nivida commented 5 years ago

I have to test it before I can say it's definitely a bug.

nivida commented 5 years ago

Tested with:

class test {
  constructor() {
    this.contract = new web3.eth.Contract(
      abi,
      '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bDD'
    );

    this.contractTwo = new web3.eth.Contract(
      abi,
      '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA'
    );

    console.log(this.contract.address);
    console.log(this.contractTwo.address);
  }
}

new test();

Result:

0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bDD
0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA

Web3.js version: 1.0.0-beta.48

ilanolkies commented 5 years ago

What about options? You are not trying that.

I'm using { defaultAccount: account }.

nivida commented 5 years ago

This does also work. Could you create a GitHub repository with an example?

Code:

class test {
  constructor() {
    this.contract = new web3.eth.Contract(
      abi,
      '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bDD',
      {defaultAccount: '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA'}
    );

    this.contractTwo = new web3.eth.Contract(
      abi,
      '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA',
      {defaultAccount: '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bDD'}
    );

    console.log(this.contract.address);
    console.log(this.contractTwo.address);
  }
}

new test();

Result:

0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bDD
0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA
ilanolkies commented 5 years ago

Yes sure!

Repo: https://github.com/ilanolkies/rns-js

Example:

import React, { Component } from 'react';
import RNS from 'rns-js';

const addresses = {
  registryAddress: '0x612322a5489929c55e2b9aacb84beef383132cda',
  publicResolverAddress: '0xdc3dad67cf3865a9b2c30381a8ca8f204d8f989e',
  registrarAddress: '0x95bfefa36f4c0504560da00c0d04fc30ef45d4c6',
  rifAddress: '0x38632564f438967e8e3259bac2f3877bb4b5104f'
};

class Resolver extends Component {
  constructor (props) {
    super(props);

    this.trigger = this.trigger.bind(this)
  }
  componentDidMount () {
    window.ethereum.enable()
    .then(accounts => {
      const account = accounts[0]
      this.rns = new RNS(window.ethereum, { defaultAccount: account }, addresses);
    });
  }

  trigger () {
    this.rns.registry.owner('z.rsk').then(console.log)
    // calling another contract here throws error, like:
    // this.rns.publicResolver.addr('z.rsk').then(console.log)
  }

  render () {
    return <div>
    <button onClick={this.trigger}>trigger</button>
    </div>
  }
}

export default Resolver;
nivida commented 5 years ago

Thanks for providing me the complete example.

I was testing it with some more classes in the meantime and was able to reproduce the behavior with the version beta.48. After that, I've tested it with the current working branch and the error disappeared. This bug got solved because I've fixed a bug regarding the options handling in the eth module Contract factory method.

This will get released asap.

Code:

class Registrar {
  constructor (web3, registrarAddress, rifAddress, options = {}) {
    this.asdf = new web3.eth.Contract(abi, registrarAddress, options);
    this.rif = new web3.eth.Contract(abi, rifAddress, options);
    console.log(this.asdf.address);
    console.log(this.rif.address);
    this.sha3 = web3.utils.sha3;
  }
}

class Wrapper {
  constructor(provider, options, addresses) {
    const web3 = new Web3(provider);
    this.registrar = new Registrar(web3, addresses.registrar, addresses.rif, options);
  }
}

const wrapper = new Wrapper(
  'http://localhost:8545',
  {defaultAccount: '0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bAA'},
  {registrar:'0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bGG',rif:'0xdf09cba4cbd5902d83d4fa4e3f5e7dbfc7bd0bKK'}
);
ilanolkies commented 5 years ago

Thanks! But this code is the same as mine... Is there a workaround or I have to wait until release?

nivida commented 5 years ago

This got fixed with PR #2466 and will be released asap. There is currently no workaround but I will release a new version this weekend or at the beginning of next week.

ilanolkies commented 5 years ago

Hi @nivida ! Was the fix released? Sory for the anxiety 😆

ilanolkies commented 5 years ago

Hi @nivida Still happening in beta.51. Please reopen this issue.

ilanolkies commented 5 years ago

Hi @nivida Still happening in beta.52. Can you please reopen this issue?

nivida commented 5 years ago

I've tested it and it worked. Could you add a GitHub repository for testing it closer? (the referenced github url points to a 404 page)

ilanolkies commented 5 years ago

Hi! Sory, we changed the url: https://github.com/rnsdomains/rns-js thanks for the time!

ilanolkies commented 5 years ago

Now it's working 😄 thanks.