Consensus fails on private txn that are intended to fail, such as revert op. #434

xuguojie · 2018-07-05T06:18:23Z

System information

Geth version: geth version

Version: 1.7.2-stable
Git Commit: 87d7c906e9eba93b618651ab5a427746506e2f30
Quorum Version: 2.0.2
Architecture: amd64
Network Id: 1
Go Version: go1.9.3
Operating System: linux
GOPATH=
GOROOT=/usr/local/go

OS & Version: Windows/Linux/OSX

vagrant box running on OSX

Branch, Commit Hash or Release: git status

Tried on both 'origin/master' and 'v2.0.2'

Expected behaviour

With Istanbul BFT, Quorum nodes should be able to reach consensus when executes a private contract that has revert/require operation. In such case, both privateFor nodes and public nodes should have consensus of the reverted transaction (status=0).

Actual behaviour

PrivateFor nodes ran the private reverting code, and marked the local tx failed (status=0), but the public nodes can't receive the private reverting code, so it ran and resulted as succeed (status=1). When the proposer (most likely to be a public node) seal a new block and propagated back to the private nodes, the privateFor nodes stuck on BAD BLOCK error.

Steps to reproduce the behaviour

Run 7nodes example, run truffle test to deploy contract as below

contract('PrivateForTest', function(accounts) {
  describe("Test Revert", function() {
    it("should be able to revert in private contract ", async function () {
      let revertContract = await RevertTest.new({privateFor: ['QfeDAys9MPDs2XHExtc84jKGHxZg/aj52DTh0vtA3Xc=']});
      await revertContract.revertFunction({privateFor: ['QfeDAys9MPDs2XHExtc84jKGHxZg/aj52DTh0vtA3Xc=']});
    });
  });
})

Solidity contracts:

pragma solidity ^0.4.18;
contract RevertTest{
   uint public newValue;
   function revertFunction() public{
       uint a = 1;
       require(a == 0);
   }
}

Backtrace

########## BAD BLOCK #########
Chain config: {ChainID: <nil> Homestead: <nil> DAO: <nil> DAOSupport: false EIP150: 2 EIP155: 3 EIP158: 3 Byzantium: 1 IsQuorum: true Engine: istanbul}

Number: 81
Hash: 0xc80285522e424ac42c81d1b5432d74195257fe5c8cc74130d810bb0e0a9a2aa5
        receipt{status=0 cgas=0 bloom=00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 logs=[]}


Error: invalid receipt root hash (remote: b64408da6b8fe39ab764af88ece1e8cca1c35fd988db57806e99138c629365a0 local: 703c035562d8e37c66f2f9461219b45c710e59c8d0d234f6b062107de627758c)
##############################

I managed to add logging to print out the tx receipts, which are different in status as below:

receipt of tx on privateFor nodes: {status=0 cgas=0 bloom=000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
000000000000000000000000000000000000000000000000 logs=[]}

receipt of tx on public nodes: {status=1 cgas=0 bloom=00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 logs=[]}

The text was updated successfully, but these errors were encountered:

xuguojie · 2018-07-06T10:44:41Z

Just an update on investigation, commenting out line 93-96 of block_validator.go helped to avoid the BAD BLOCK issue and chain seem to generate blocks fine, although there might be potential impact of the having inconsistent tx receipts between nodes.

jmc265 · 2018-07-19T11:22:26Z

I am seeing the same thing.
If you do a transaction that has 'privateFor' as part of the parameters, and that tranasction fails, the other nodes will reject the block and will drop the node that created the block.

AldelopiomicA · 2018-08-09T06:41:02Z

from the slack @fixanoid asked.

I can confirm this but I'm running in normal raft.

System information
using 7nodes vagrant example but update and built the quorum once

Version: 1.7.2-stable
Git Commit: >43424382f29ec3472941fa9de014a41344e5ec55
Quorum Version: 2.0.2
Architecture: amd64
Network Id: 1
Go Version: go1.9.3
Operating System: linux
GOPATH=
GOROOT=/usr/local/go

Expected Behavior
System should revert transaction like nothing happen when the code fail required(failedConfition); in solidity contract

Actual Behavior
The node crash when such transaction happen. And cannot restart again.

Step to Reproduce

Run raft-init.sh and raft-start.sh
get into one node1 geth and deploy the contract with

var initVal = 10 ;
var simplestorageContract = web3.eth.contract([{"constant":true,"inputs":[],"name":"storedData","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"x","type":"uint256"}],"name":"set","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"get","outputs":[{"name":"retVal","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"x","type":"uint256"}],"name":"setMoreThan2","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"inputs":[{"name":"initVal","type":"uint256"}],"payable":false,"stateMutability":"nonpayable","type":"constructor"}]);
var simplestorage = simplestorageContract.new(
   initVal,
   {
     from: web3.eth.accounts[0], 
     data: '0x608060405234801561001057600080fd5b50604051602080610140833981016040525160005561010c806100346000396000f300608060405260043610605c5763ffffffff7c01000000000000000000000000000000000000000000000000000000006000350416632a1afcd98114606157806360fe47b11460855780636d4ce63c14609c578063a3d811751460ae575b600080fd5b348015606c57600080fd5b50607360c3565b60408051918252519081900360200190f35b348015609057600080fd5b50609a60043560c9565b005b34801560a757600080fd5b50607360ce565b34801560b957600080fd5b50609a60043560d4565b60005481565b600055565b60005490565b6002811160c957600080fd00a165627a7a723058200d23f33b0efe56f383e96897145fd37a86a9c15d488da8be19682d2063b2948f0029', 
     gas: '4700000',
     privateFor:["oNspPPgszVUFw0qmGFfWwh1uxVUXgvBxleXORHj07g8="]
   }, function (e, contract){
    console.log(e, contract);
    if (typeof contract.address !== 'undefined') {
         console.log('Contract mined! address: ' + contract.address + ' transactionHash: ' + contract.transactionHash);
    }
 })

Note for the solidity contract

pragma solidity ^0.4.15;

contract simplestorage {
  uint public storedData;

  constructor(uint initVal) public {
    storedData = initVal;
  }

  function set(uint x) public {
    storedData = x;
  }
  
  function setMoreThan2(uint x) public {
      require(x > 2);
      storedData = x;
  }

  function get() public constant returns (uint retVal)  {
    return storedData;
  }
}

continue try the function setMoreThan2(uint x) with some value that will break like simplestorage.setMoreThan2(1, {from: web3.eth.accounts[0], gas: '4700000', privateFor:["oNspPPgszVUFw0qmGFfWwh1uxVUXgvBxleXORHj07g8="]});
see log for error

########## BAD BLOCK #########
Chain config: {ChainID: 10 Homestead: <nil> DAO: <nil> DAOSupport: false EIP150: 1 EIP155: 0 EIP158: 1 Byzantium: 1 IsQuorum: true Engine: unknown}

Number: 4
Hash: 0x9ab9e3719812a6c42d50e94701105600697f74a7b0306da0459bb8cc94953276
        receipt{status=0 cgas=0 bloom=00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 logs=[]}


Error: invalid receipt root hash (remote: b64408da6b8fe39ab764af88ece1e8cca1c35fd988db57806e99138c629365a0 local: 703c035562d8e37c66f2f9461219b45c710e59c8d0d234f6b062107de627758c)
##############################

panic: failed to extend chain: invalid receipt root hash (remote: b64408da6b8fe39ab764af88ece1e8cca1c35fd988db57806e99138c629365a0 local: 703c035562d8e37c66f2f9461219b45c710e59c8d0d234f6b062107de627758c)

goroutine 195 [running]:
github.com/ethereum/go-ethereum/raft.(*ProtocolManager).applyNewChainHead(0xc42020fce0, 0xc43567aa20)
        /home/vagrant/quorum/build/_workspace/src/github.com/ethereum/go-ethereum/raft/handler.go:876 +0x898
github.com/ethereum/go-ethereum/raft.(*ProtocolManager).eventLoop(0xc42020fce0)
        /home/vagrant/quorum/build/_workspace/src/github.com/ethereum/go-ethereum/raft/handler.go:730 +0x5e5
created by github.com/ethereum/go-ethereum/raft.(*ProtocolManager).startRaft
        /home/vagrant/quorum/build/_workspace/src/github.com/ethereum/go-ethereum/raft/handler.go:485 +0x789

fixanoid · 2018-08-09T14:14:39Z

Preliminary tests confirm that this is a bug in privacy core.

Previously we had populated the public receipt `failed` field with the result of the transaction. This is correct for public transactions. It's also correct for successful private transactions. But it's not correct for failing private transactions, because their public receipt should not indicate failure. The fix is straightforward. Testing: I used this contract: contract RevertTest{ uint public newValue; function revertFunction() public{ uint a = 1; require(a == 0); } } After deploying the contract I sent in several failing transactions via function sendBad() { eth.sendTransaction({ from: eth.accounts[0], data: web3.sha3("revertFunction()"), gas: 0x47b760, privateFor: ["ROAZBWtSacxXQrOe3FGAqJDyJjFePR5ce4TSIzmJ0Bc="] }); } Watching the logs (`1.log` and `2.log`), I saw the `TX-ACCEPTED` events scroll as I sent `revertFunction` transactions. I see 10 `TX-ACCEPTED` events in both logs (1 for deploy and 9 tests via `sendBad`). Via extra logging, in `1.log` I see that the public receipts have status `1`, whereas private receipts have status `0`. In `2.log` they all have status `1`. All nodes stayed up the whole time. Fixes #434

joelburget · 2018-09-08T23:43:22Z

Hi @xuguojie, @jmc265, and @AldelopiomicA. I proposed a fix above (#517). I'd love to know whether it fixes the problem for you.

xuguojie · 2018-09-08T23:45:39Z

Thanks, @joelburget. Will take a look. :)

AldelopiomicA · 2018-09-10T07:21:33Z

Sorry to ask simple question. But how to test it?
I've try
vagrant ssh
Then inside the vagrant

cd quorum
git fetch origin pull/517/head:private-revert-fix
git checkout private-revert-fix

Then this is part I'm not to sure

make clean
rm -fr build/_workspace/pkg/ /home/vagrant/quorum/build/bin/*
make all

Then I proceed to test at the quorum example. But then there is still crash happen. I check geth version it still show the same commit as I've post so I'm not sure whether I built and install the quorum correctly. Or missing some step.

vagrant@ubuntu-xenial:~/quorum-examples/7nodes$ geth version
Geth
Version: 1.7.2-stable
Git Commit: 43424382f29ec3472941fa9de014a41344e5ec55
Quorum Version: 2.0.2
Architecture: amd64
Network Id: 1
Go Version: go1.9.3
Operating System: linux
GOPATH=
GOROOT=/usr/local/go

fixanoid · 2018-09-10T16:02:17Z

@AldelopiomicA all the commands are correct. Last step is to overwrite geth executable, after make all, like so:
sudo cp build/bin/geth /usr/local/bin

AldelopiomicA · 2018-09-11T02:53:20Z

Follow @fixanoid now I've test the PR. And it does fix the problem for me.

I've test with what I've describe here. And also test on my project which using a lot of revert. And so far it doesn't fail/crash.

@joelburget @fixanoid Thank you very much.

The geth version are now

Version: 1.7.2-stable
Git Commit: 1b301d5ead2ba9d0b5d601697221ee8731350a96
Quorum Version: 2.1.0
Architecture: amd64
Network Id: 1337
Go Version: go1.9.3
Operating System: linux
GOPATH=
GOROOT=/usr/local/go

Fix consensus on private contract failure. Fixes #434

fixanoid added the needs review label Jul 24, 2018

fixanoid added bug consensus core privacy and removed needs review labels Aug 9, 2018

fixanoid mentioned this issue Aug 14, 2018

Run 7 nodes example,only 6 geth can be run. #483

Closed

jpmsam assigned bts, joelburget and jpmsam Aug 28, 2018

prd-fox mentioned this issue Sep 4, 2018

The geth will auto exit #514

Closed

joelburget mentioned this issue Sep 8, 2018

Fix consensus on private contract failure. #517

Merged

fixanoid changed the title ~~IBFT consensus failed on private contract with revert operation~~ Consensus fails on private txn that are intended to fail, such as revert op. Sep 10, 2018

jpmsam closed this as completed in #517 Sep 18, 2018

jpmsam added a commit that referenced this issue Sep 18, 2018

Merge pull request #517 from jpmorganchase/private-revert-fix

2131e96

Fix consensus on private contract failure. Fixes #434

SatpalSandhu61 mentioned this issue Oct 9, 2018

Getting bad block exception in geth logs and geth quits #548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consensus fails on private txn that are intended to fail, such as revert op. #434

Consensus fails on private txn that are intended to fail, such as revert op. #434

xuguojie commented Jul 5, 2018

xuguojie commented Jul 6, 2018

jmc265 commented Jul 19, 2018

AldelopiomicA commented Aug 9, 2018

fixanoid commented Aug 9, 2018

joelburget commented Sep 8, 2018

xuguojie commented Sep 8, 2018

AldelopiomicA commented Sep 10, 2018 •

edited

Loading

fixanoid commented Sep 10, 2018

AldelopiomicA commented Sep 11, 2018 •

edited

Loading

Consensus fails on private txn that are intended to fail, such as revert op. #434

Consensus fails on private txn that are intended to fail, such as revert op. #434

Comments

xuguojie commented Jul 5, 2018

System information

Expected behaviour

Actual behaviour

Steps to reproduce the behaviour

Backtrace

xuguojie commented Jul 6, 2018

jmc265 commented Jul 19, 2018

AldelopiomicA commented Aug 9, 2018

I can confirm this but I'm running in normal raft.

fixanoid commented Aug 9, 2018

joelburget commented Sep 8, 2018

xuguojie commented Sep 8, 2018

AldelopiomicA commented Sep 10, 2018 • edited Loading

fixanoid commented Sep 10, 2018

AldelopiomicA commented Sep 11, 2018 • edited Loading

AldelopiomicA commented Sep 10, 2018 •

edited

Loading

AldelopiomicA commented Sep 11, 2018 •

edited

Loading