consensus/istanbul: Istanbul BFT (EIP650) #14674

yutelin · 2017-06-22T08:22:12Z

This PR is the implementation of Istanbul BFT protocol in ethereum/EIPs#650

fjl · 2017-06-22T20:31:27Z

Warning: it will take a while to get this merged ;)

GitCop · 2017-08-31T03:16:17Z

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

Commit: 2620c98
Commits must be prefixed with the package(s) they modify
Commit: 7e94f76
Commits must be prefixed with the package(s) they modify
Commit: 3f18431
Commits must be prefixed with the package(s) they modify
Commit: 687d5ca
Commits must be prefixed with the package(s) they modify
Commit: edcbde3
Commits must be prefixed with the package(s) they modify
Commit: bd88b3e
Commits must be prefixed with the package(s) they modify
Commit: 165e40b
Commits must be prefixed with the package(s) they modify
Commit: 214c4e1
Commits must be prefixed with the package(s) they modify
Commit: 6637498
Commits must be prefixed with the package(s) they modify
Commit: ef7b0fc
Commits must be prefixed with the package(s) they modify
Commit: 1f7da34
Commits must be prefixed with the package(s) they modify
Commit: f81f870
Commits must be prefixed with the package(s) they modify
Commit: 13c4671
Commits must be prefixed with the package(s) they modify
Commit: 1550260
Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.

This message was auto-generated by https://gitcop.com

stevenroose · 2017-11-10T11:56:44Z

--istanbul.blockpausetime value Pause time when zero tx in previous block, values should be larger than istanbul.blockperiod (default: 2)
I guess it's a good idea to fail when that is not the case :)

stevenroose · 2017-11-10T14:16:11Z

consensus/istanbul/backend/backend_test.go

+	}
+}
+
+/**


Forgotten comment?

kesar · 2017-12-14T16:35:15Z

could conflicts be solved? I would love to use this algo!

stevenroose · 2018-01-24T10:21:50Z

-- repost of comment on EIP 650 --

The current implementation (as found in Quorum) breaks the concept of the "pending" block, used in several calls, but most notably in eth_getTransactionCount (PendingNonceAt in ethclient):

In Ethereum, the pending block means the latest confirmed block + all pending transactions the node is aware of. This means that directly after a transaction is sent to the node (through RPC), the transaction count (aka nonce) in the "pending" block is increased. A lot of tools, like abigen in this repo or any other tool where tx signing occurs at the application level instead of in geth, rely on this for making multiple transactions at once. After the first one, the result of eth_getTransactionCount will increase so that a valid second tx can be crafted.

With the current implementation of Istanbul, the definition of the "pending block" seem to be different. When submitting a transaction, the result for eth_getTransactionCount for the sender in the "pending" block does not change. When a new block is confirmed (not containing this tx), it does change however (while the value for "latest" doesn't). Then, on the next block confirmation, the "latest" also changes because the tx is in the confirmed block.

So this seems to mean that the "pending block" definition changed from "latest block + pending txs" to "the block that is currently being voted on". I consider this a bug; if this is done on purpose, it breaks with a lot of existing applications (all users of abigen, f.e.) and should be reconsidered.

I originally reported about this issue in the Quorum repo, but there doesn't seem to be a good place to report bugs in Istanbul other than here.

AntoineRondelet · 2018-03-21T15:49:13Z

consensus/consensus.go

+	Protocol() Protocol
+}
+
+// Handler should be implemented is the consensus needs to handle and send peer's message


Isn't it "if the consensus" instead of "is the consensus" ?

AntoineRondelet · 2018-03-21T16:20:34Z

consensus/istanbul/backend.go

+	LastProposal() (Proposal, common.Address)
+
+	// HasPropsal checks if the combination of the given hash and height matches any existing blocks
+	HasPropsal(hash common.Hash, number *big.Int) bool


I would change it to HasProposal to keep it consistent with the rest

AntoineRondelet · 2018-03-21T16:24:23Z

consensus/istanbul/backend/backend.go

+)
+
+const (
+	// fetcherID is the ID indicates the block is from Istanbul engine


I would say "fetcher ID indicates that the block..."

AntoineRondelet · 2018-03-21T16:29:51Z

consensus/istanbul/backend/backend.go

+
+// ----------------------------------------------------------------------------
+
+type backend struct {


I would rather get rid of the // --------------------- comment line and move the struct before the New function (which would make sense because we use the backend struct in the New function) for clarity/cleanliness.

AntoineRondelet · 2018-03-21T16:33:00Z

consensus/istanbul/backend/backend.go

+	return nil
+}
+
+// Broadcast implements istanbul.Backend.Gossip


Isn't is Gossip, rather than Broadcast ?

AntoineRondelet · 2018-03-21T16:56:11Z

consensus/istanbul/core/backlog.go

+		if err == nil {
+			backlog.Push(msg, toPriority(msg.Code, p.View))
+		}
+		// for msgRoundChange, msgPrepare and msgCommit cases


Indentation problem, or is this expected ?

AntoineRondelet · 2018-03-21T17:02:06Z

consensus/istanbul/core/handler.go

+	return nil
+}
+
+// ----------------------------------------------------------------------------


I would remove this comment

AntoineRondelet · 2018-03-21T17:03:49Z

consensus/istanbul/core/message_set.go

+	}
+}
+
+// ----------------------------------------------------------------------------


Useless comment ?

AntoineRondelet · 2018-03-21T17:04:35Z

consensus/istanbul/core/message_set.go

+	return ms.messages[addr]
+}
+
+// ----------------------------------------------------------------------------


Useless comment ?

AntoineRondelet · 2018-03-21T17:11:21Z

consensus/istanbul/core/roundstate.go

+}
+
+// roundState stores the consensus state
+type roundState struct {


I would move this struct just before the newRoundState function

…to support custom messages

kaidiren · 2018-06-27T12:40:47Z

cool, please go ahead

adamschmideg · 2019-01-10T09:49:47Z

We don't have the capacity to merge and maintain it. We had an informal chat with Quorum, and they might write the integration.

osouza-de · 2019-02-18T13:47:14Z

Is there any update regarding the integration?

jpmsam · 2019-02-18T15:40:26Z

It is integrated in Quorum which is a fork of Geth.

osouza-de · 2019-02-18T15:47:12Z

It is integrated in Quorum which is a fork of Geth.

Yes, I'm aware of it.
But I mean when it's going to be merged into Ethereum geth official repository.

As we can see in the comment:

We don't have the capacity to merge and maintain it. We had an informal chat with Quorum, and they might write the integration.

nambrot · 2019-03-01T22:54:45Z

consensus/istanbul/backend/engine.go

+	}
+
+	for _, seal := range committedSeals {
+		if len(seal) != types.IstanbulExtraSeal {


Could someone tell me what this checks? I was trying to figure out where we check during the commit/finalization that the commit seals are valid?

stale · 2020-03-01T22:55:29Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

itsdevbear · 2021-12-01T06:13:03Z

wen merge >:(

karalabe · 2021-12-02T09:51:43Z

never

zhiqiangxu · 2022-03-16T23:42:24Z

never

@karalabe Is there an official reason for this?

sarthaktNt · 2023-01-18T10:59:31Z

Polygon Edge migration from POA to POS

I have created a 4 nodes POA network using polygon-edge.
As mentioned ,I have tried out the approach for migration.

I have deployed the staking smart contract in block no 59357 .All the validators staking has staked before block no 59750.I have verified the list in staking contract.
I have restarted all the nodes after migration to rebuild the blocks from new genesis.json.
polygon-edge ibft switch --chain ./genesis.json --type PoS --deployment 59357 --from 59750
But the blocks are not getting generated from 59750 due to the following error.

failed to get validators when calculation quorum

Please guys help me to resolve my issue

karalabe · 2023-01-18T11:01:03Z

This is a repo for Ethereum, not Polygon.

yutelin mentioned this pull request Jun 22, 2017

Istanbul Byzantine Fault Tolerance ethereum/EIPs#650

Closed

fjl changed the title ~~Istanbul Byzantine Fault Tolerance~~ consensus/istanbul: Istanbul Byzantine Fault Tolerance (EIP650) Jun 22, 2017

fjl changed the title ~~consensus/istanbul: Istanbul Byzantine Fault Tolerance (EIP650)~~ consensus/istanbul: Istanbul BFT (EIP650) Jun 22, 2017

markya0616 force-pushed the feature/istanbul branch from e76a5b5 to 7207cbc Compare July 17, 2017 08:03

tailingchen force-pushed the feature/istanbul branch from 68cbcf9 to 74ccc93 Compare July 31, 2017 09:58

markya0616 force-pushed the feature/istanbul branch 2 times, most recently from 41dfb17 to 0f066fb Compare August 7, 2017 15:07

markya0616 force-pushed the feature/istanbul branch 2 times, most recently from ee11f70 to 042d72e Compare August 31, 2017 03:16

markya0616 force-pushed the feature/istanbul branch from 042d72e to ee11f70 Compare August 31, 2017 03:19

markya0616 force-pushed the feature/istanbul branch 2 times, most recently from ee46a57 to 2df6174 Compare September 15, 2017 07:09

markya0616 force-pushed the feature/istanbul branch 2 times, most recently from 64789fb to b612e3b Compare October 4, 2017 03:04

markya0616 force-pushed the feature/istanbul branch from b612e3b to 6df88ce Compare November 9, 2017 03:27

stevenroose reviewed Nov 10, 2017

View reviewed changes

consensus/istanbul/backend/backend_test.go

}

}

/**

Copy link

Contributor

stevenroose Nov 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgotten comment?

markya0616 force-pushed the feature/istanbul branch from 6df88ce to 5fe3ba1 Compare November 23, 2017 03:30

yutelin requested review from holiman and karalabe as code owners November 23, 2017 03:30

markya0616 force-pushed the feature/istanbul branch from 5fe3ba1 to 880a89f Compare December 18, 2017 02:51

markya0616 force-pushed the feature/istanbul branch 2 times, most recently from 1f50b64 to 96e5059 Compare January 23, 2018 01:58

AntoineRondelet reviewed Mar 21, 2018

View reviewed changes

countvonzero mentioned this pull request Mar 26, 2018

consensus, eth: Istanbul interface proposal #16385

Closed

cmd, consensus, eth, ethstats: add protocol interface into consensus …

e22d86f

…to support custom messages

markya0616 and others added 9 commits March 26, 2018 17:31

consensus/istanbul: common Istanbul interfaces and types

d9b440a

consensus/istanbul: Istanbul validator set implementation

f03a83a

consensus/istanbul: Istanbul core implementation

0abccdb

consensus/istanbul: add tests for Istanbul core

1e297ef

core/types: add Istanbul specific block hash calculation

efd8b5b

consensus/istanbul: Istanbul consensus backend implementation

72ae3ec

internal/web3ext: add Istanbul JS RPC API

3718bc0

core, les, eth, miner: Istanbul consensus integration

159fba7

cmd/*, core, params: add ottoman testnet

c754738

markya0616 force-pushed the feature/istanbul branch from 96e5059 to c754738 Compare March 26, 2018 09:42

yutelin requested a review from zsfelfoldi as a code owner March 26, 2018 09:42

fjl added the eip label Sep 5, 2018

adamschmideg assigned fjl Dec 7, 2018

adamschmideg added the status:on-hold label Dec 7, 2018

nambrot reviewed Mar 1, 2019

View reviewed changes

nambrot mentioned this pull request Mar 16, 2019

Don't propose empty blocks with Istanbul celo-org/celo-blockchain#134

Merged

2 tasks

stale bot added the status:inactive label Mar 1, 2020

stale bot closed this Apr 12, 2020

IronGauntlets mentioned this pull request Apr 22, 2020

Tdm refactor2 autonity/autonity#512

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consensus/istanbul: Istanbul BFT (EIP650) #14674

consensus/istanbul: Istanbul BFT (EIP650) #14674

yutelin commented Jun 22, 2017

fjl commented Jun 22, 2017

GitCop commented Aug 31, 2017

stevenroose commented Nov 10, 2017

stevenroose Nov 10, 2017

kesar commented Dec 14, 2017

stevenroose commented Jan 24, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

AntoineRondelet Mar 21, 2018

kaidiren commented Jun 27, 2018

adamschmideg commented Jan 10, 2019

osouza-de commented Feb 18, 2019

jpmsam commented Feb 18, 2019

osouza-de commented Feb 18, 2019 •

edited

Loading

nambrot Mar 1, 2019

stale bot commented Mar 1, 2020

itsdevbear commented Dec 1, 2021

karalabe commented Dec 2, 2021

zhiqiangxu commented Mar 16, 2022

sarthaktNt commented Jan 18, 2023

karalabe commented Jan 18, 2023

+              	}
+              }
+              /**


		// ----------------------------------------------------------------------------

		type backend struct {

consensus/istanbul: Istanbul BFT (EIP650) #14674

consensus/istanbul: Istanbul BFT (EIP650) #14674

Conversation

yutelin commented Jun 22, 2017

fjl commented Jun 22, 2017

GitCop commented Aug 31, 2017

stevenroose commented Nov 10, 2017

Choose a reason for hiding this comment

kesar commented Dec 14, 2017

stevenroose commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaidiren commented Jun 27, 2018

adamschmideg commented Jan 10, 2019

osouza-de commented Feb 18, 2019

jpmsam commented Feb 18, 2019

osouza-de commented Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

stale bot commented Mar 1, 2020

itsdevbear commented Dec 1, 2021

karalabe commented Dec 2, 2021

zhiqiangxu commented Mar 16, 2022

sarthaktNt commented Jan 18, 2023

karalabe commented Jan 18, 2023

osouza-de commented Feb 18, 2019 •

edited

Loading