14

I don't understand why if i execute my function with web3.js i obtain this error in the transaction:

Warning! Error encountered during contract execution [Bad instruction]

I read this other question about the same error. I don't understand why with this code its happening the same:

contract Music is owned{

    string public themeMusic;
    string public idMusic;
    int public money;

    function Music (string setThemeMusic, string setIdMusic, int setmoney) {
        themeMusic = setThemeMusic;
        idMusic = setIdMusic;
        money = setmoney;
    }

    function setMoney(int moneyUpdate) onlyOwner {
        money = moneyUpdate;
    }
}

The error is with the setMoney function, is for the onlyOwner? i need to make it payable anyway? I don't want to make it payable, just only use it for set the money and this function only for the owned.

More information:

  • I make other contracts with similar functions and don't have this problem.
  • This contract is created by other contract like the typical example Factory.
  • Owned is a contrac where i store the address and test if is the owner.
  • I'm using Meteor.js and Web3.js.

How do you interact with the contract? For interact with the contract i'm using the Web3.js APi of ethereum.

What command do you use? I'm defining the contract with the ABI and the address of the contract like this myContract = web3.eth.contract(ABIArray).at(contractAddress); and then i call the function of the contract like this myContract.setMoney(money); (and the callback function).

Do you use production blockchain? I'm working in Ropsten Test-net.

My Music company:

contract musicCompany is owned {

    address[] public listofmusic;

    function addMusic(string setThemeMusic, string setIdMusic, int setmoney) onlyOwner {
        address newMusic = new Music(setThemeMusic, setIdMusic, setmoney);
        listofmusic.push(newMusic);
    }
}

This contract can create a new Music contracts.

My Owned contract:

contract owned {

    address public owner;

    function owned() {
        owner = msg.sender;
    }

    modifier onlyOwner {
        if (msg.sender != owner) throw;
        _;
    }

}

My Web3.js code:

'click .setMoney'(event, instance) {
     var price = document.getElementById('priceUpdate').value;
     event.preventDefault();
     alert("contract price");
     myContract.setMoney(price, function(error, result){
      if(!error)
          console.log(result);
      else
          console.error(error);
      })

Screenshot of the metamask transactions without any problem:

Screenshot of the metamask transactions without any problem

Screenshot of the EtherScan.io transaction with the error: Screenshot of the EtherScan.io transaction with the error:

Update 1: I try using unsigned integers and bytes32 instead of int, the result is the same. Maybe the problem is in the modifier, could it be?

Update 2: I tried to delete the onlyOwnermodifier of the function and doesn't work ...

Update 3: I tried to delete all the modifier, is owned and onlyOwner and still doesn't working. I think the problem is with the builder function, i will starting now to make test with it...

Update 4: By doing Test, i know that moneyUpdate have the value correctly and money the same, but I have discovered that the value of money don't change. And I have discovered too that when i print the result variable of my callback inside "My Web3.js code" i obtain the adress of the transaction 0x0fda5d31d02c87aa20c45a54f6859c7576b4f0aae4a639c47ec23e5c4e‌​3d9953. In principle, the transaction is performed but the error prevents the change.

Some transactions (all are mine) for view the error:

0x52ef9164750c4e174cba1a5f45343220458f1cedca25380f0aaf102df0a8f895

0x4e00d9d67ffb906e0036434ea2e14acea82a6aabb4d8c99d94c0b2bcdef24f7a

0x4663fa874993d54989d073f72d6846cae652a454349e62882dccc96d78528a8e

0x69fbf54ffd81ee114efcd750d4e97bb528ecc868140abdecf218e9ae66391a5

Gawey
  • 814
  • 2
  • 8
  • 23
  • Can you provide a link to one of the failing transactions? – DeviateFish Jun 08 '17 at 19:48
  • yes you have one in the answer, but i can put more. – Gawey Jun 09 '17 at 09:56
  • Generally, I recommend using the Truffleframework for development. https://github.com/trufflesuite/truffle It eliminates many problems like these right away and helps you to get going more fluently. – n1cK Jun 09 '17 at 10:35
  • @NikitaFuchs Yes, i know the framework, but this development started with meteor... Now I can't change or is there easy migration? – Gawey Jun 09 '17 at 11:12
  • 1
    With truffle, you get promisified functions according to the function names in your contract, it doesn't get any simpler. – n1cK Jun 09 '17 at 13:35

4 Answers4

7

The code is so obviously simple that the only problem would seem to be with onlyOwner. I would check the value of the owner variable. I bet it's not what you think it is. (It's either zero or the value of the sender who deployed the contract). You didn't provide the address of the contract, so I can't check the value of owner on Etherscan. Plus you're not providing the address of the calling account. If they don't match, the transaction will throw, which (as is true of all Ethereum errors) will report out of gas. If the code is really as simple as you say, the only possible problem is with owner. I would check that.

Thomas Jay Rush
  • 9,943
  • 4
  • 31
  • 72
  • I will try it and then i comment something, i put some transactions on the question if you want check this. – Gawey Jun 09 '17 at 10:03
  • I delete all reference to the owner class in this contract and in the contract who created the music contract, and it works but if i put again it fail... What is happening? – Gawey Jun 09 '17 at 12:38
  • During initialization the owner variable is either getting set to a account address different than the one you think it is, or it's not getting set at all (and is therefore zero). You should be able to tell by using the 'ReadContract' tab on etherscan.io.

    Someone commented above that there was no constructor in your code which I would think means the 'owner' is zero. What is the address? I can check it.

    – Thomas Jay Rush Jun 09 '17 at 16:44
  • You need to provide the full source code for the contract, or a pointer to it via GitHub or something. If it works without the isOwner, and doesn't work with it, then the value of owner is not set to the msg.sender. (If you look at the owner contract you can see that isOwner only throws if the values don't match. The reason for this is because you're most likely not setting owner. Maybe I'm missing something. See the actual full source would help. – Thomas Jay Rush Jun 12 '17 at 02:47
  • Yes i fixed it, with your tip the address set in 'isOwner' is not the same of the contract Music so when i use 'onlyOwner' modifier this don't work, cause this address is the parent contract. This fail makes me modify all the code a lot of times... for just a wrong address. – Gawey Jun 12 '17 at 11:04
  • The owner gets set in the constructor using the msg.sender, which is the address of the account that deployed the contract. If you're calling into a function protected by isOwner later, and the address that is initiating that call is not the same as the one who deployed the contract it will throw an error. It sounds to me that you may have chosen to remove the isOwner which is maybe not the best idea. You should call from the proper owner otherwise you lose the protection that the owned contract provides. Thanks for the bounty. – Thomas Jay Rush Jun 12 '17 at 12:33
  • No don't worry about isOwner, i'm testing for fix it the problem with the isOwner but my principal problem "Identify where the problem is here and why this happens" is solved. Now i have other problem with the addres who execute the function and why is not the same of the other. I will put the last part of my code for if you have some idea that was is wrong with that. – Gawey Jun 12 '17 at 12:54
  • Updated with last part of the contracts interaction, that's all. If you have some idea can answer me with a comment. (Maybe the problem was the creation of the contract, i'm testing now about it) – Gawey Jun 12 '17 at 13:01
  • 1
    At this point I would suggest that you greatly simplify the question and ask a new question and they will get more answers from other people. – Thomas Jay Rush Jun 12 '17 at 13:04
3

Your transaction appears to be failing because you're calling it from a different address than you created it with. I walked through the first transaction and noticed that it was skipping an equals test (ISZERO + JUMPI) when comparing against the caller. Sure enough, that transaction was sent from 0x178597064fc55ea505c3ea066c154d92bc264943, but the contract was created by 0xc2351205ee14cdb8b61ffc826ad1906bc7c08755

So, as Thomas posted, the problem is indeed due to the onlyOwner modifier.

It would be helpful to see the code for the owned contract from which yours is inheriting. I'd imagine it doesn't do much more than set owner to msg.sender, but there might be some additional complications in there.

Regardless, if you try sending from 0xc2351205ee14cdb8b61ffc826ad1906bc7c08755 to the contract in the first transaction, it should work.

DeviateFish
  • 1,128
  • 5
  • 11
2

In addition to more details that would be nice (How do you interact with the contract? What command do you use?), you need to add a constructor function to the contract. Also, try using unsigned integers instead of int.

contract Music is owned{

    string public themeMusic;
    string public idMusic;
    int public money;

    function Music() {
    // this is the constructor, do something here or leave it empty
    }
    function Car (string setThemeMusic, string setIdMusic, uint setmoney) {
        themeMusic = setThemeMusic;
        idMusic = setIdMusic;
        money = setmoney;
    }

    function setMoney(uint moneyUpdate) onlyOwner {
        money = moneyUpdate;
    }
n1cK
  • 3,378
  • 2
  • 12
  • 18
  • Sorry car is not the correct name of the function, car is the Music constructor, who is called for other function in other contract who generate the contracts. I fix it in the question. – Gawey Jun 07 '17 at 07:17
1

I tested your code and it works fine (I've put the owned part to the same file). I used truffle with testrpc (I wrote and run a JS test). Maybe the problem is how you deploy/interact with your contract.

Side note: function needs to be payable when you send Ether by calling it (you want to receive Ether by function invocation). Here you don't send real money, you just set the value.

Lukasz Zuchowski
  • 573
  • 2
  • 11
  • I'm using meteor.js and this structure works fine in other of my contracts, with the same set function and the same functions of the web3.js API, and here when i execute this function ( cause only don't work this function, the others functions work's well) appear this error in etherscan and the transaction is not processed well so i can't change the money. – Gawey Jun 07 '17 at 13:59
  • Are you sure that price you send to setMoney is an integer value? Have you tried with hardcoded value e.g. 10 ? – Lukasz Zuchowski Jun 07 '17 at 15:34
  • Hey Lukasz i try to hardcode the value, and happen the same, but when i use the Ethereum scan for view the transaction, i obtained in the data input the number but it is in hexadecimal thats is normal? like this: [0]:000000000000000000000000000000000000000000000000000000000000000a – Gawey Jun 08 '17 at 07:35
  • @Gawey I really don't know. Try the procedure below.
    1. What does the callback function print out?
    2. Try to emit event (that will contain value of money and moneyUpdate) after setting the value and there capture the event in the browser and print it out.
    3. Try to read the money value from contract. Maybe it's was updated and it's just Etherscan bug.
    4. Do you use production blockchain?
    – Lukasz Zuchowski Jun 08 '17 at 08:11
  • for the 2) and 3) question all is well, moneyUpdate have the value correctly and money the same, but I have discovered that the value of money don't change. For the 4) i 'm working in Ropsten test-net. For the 1) I have discovered that when i print the result variable of my callback inside "My Web3.js code" i obtain the adress of the transaction 0x0fda5d31d02c87aa20c45a54f6859c7576b4f0aae4a639c47ec23e5c4e3d9953, some idea? In principle, the transaction is performed but the error prevents the change. – Gawey Jun 08 '17 at 10:22
  • No better idea for now. Just let me know when you fix it, I'm curious what was wrong. – Lukasz Zuchowski Jun 08 '17 at 11:17
  • Ok, thank you i will offer a bounty for the answer hahahaha – Gawey Jun 08 '17 at 14:00