Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: implement simple checkpoint syncing #19543

Merged
merged 11 commits into from Jun 28, 2019

Conversation

Projects
None yet
5 participants
@rjl493456442
Copy link
Member

commented May 9, 2019

This PR is the successor of #17578

New version contract is

pragma solidity ^0.5.10;

/**
 * @title CheckpointOracle
 * @author Gary Rong<garyrong@ethereum.org>, Martin Swende <martin.swende@ethereum.org>
 * @dev Implementation of the blockchain checkpoint registrar.
 */
contract CheckpointOracle {
    /*
        Events
    */

    // NewCheckpointVote is emitted when a new checkpoint proposal receives a vote.
    event NewCheckpointVote(uint64 indexed index, bytes32 checkpointHash, uint8 v, bytes32 r, bytes32 s);

    /*
        Public Functions
    */
    constructor(address[] memory _adminlist, uint _sectionSize, uint _processConfirms, uint _threshold) public {
        for (uint i = 0; i < _adminlist.length; i++) {
            admins[_adminlist[i]] = true;
            adminList.push(_adminlist[i]);
        }
        sectionSize = _sectionSize;
        processConfirms = _processConfirms;
        threshold = _threshold;
    }

    /**
     * @dev Get latest stable checkpoint information.
     * @return section index
     * @return checkpoint hash
     * @return block height associated with checkpoint
     */
    function GetLatestCheckpoint()
    view
    public
    returns(uint64, bytes32, uint) {
        return (sectionIndex, hash, height);
    }

    // SetCheckpoint sets  a new checkpoint. It accepts a list of signatures
    // @_recentNumber: a recent blocknumber, for replay protection
    // @_recentHash : the hash of `_recentNumber`
    // @_hash : the hash to set at _sectionIndex
    // @_sectionIndex : the section index to set
    // @v : the list of v-values
    // @r : the list or r-values
    // @s : the list of s-values
    function SetCheckpoint(
        uint _recentNumber,
        bytes32 _recentHash,
        bytes32 _hash,
        uint64 _sectionIndex,
        uint8[] memory v,
        bytes32[] memory r,
        bytes32[] memory s)
        public
        returns (bool)
    {
        // Ensure the sender is authorized.
        require(admins[msg.sender]);

        // These checks replay protection, so it cannot be replayed on forks,
        // accidentally or intentionally
        require(blockhash(_recentNumber) == _recentHash);

        // Ensure the batch of signatures are valid.
        require(v.length == r.length);
        require(v.length == s.length);

        // Filter out "future" checkpoint.
        if (block.number < (_sectionIndex+1)*sectionSize+processConfirms) {
            return false;
        }
        // Filter out "old" announcement
        if (_sectionIndex < sectionIndex) {
            return false;
        }
        // Filter out "stale" announcement
        if (_sectionIndex == sectionIndex && (_sectionIndex != 0 || height != 0)) {
            return false;
        }
        // Filter out "invalid" announcement
        if (_hash == ""){
            return false;
        }

        // EIP 191 style signatures
        //
        // Arguments when calculating hash to validate
        // 1: byte(0x19) - the initial 0x19 byte
        // 2: byte(0) - the version byte (data with intended validator)
        // 3: this - the validator address
        // --  Application specific data
        // 4 : checkpoint section_index(uint64)
        // 5 : checkpoint hash (bytes32)
        //     hash = keccak256(checkpoint_index, section_head, cht_root, bloom_root)
        bytes32 signedHash = keccak256(abi.encodePacked(byte(0x19), byte(0), this, _sectionIndex, _hash));

        address lastVoter = address(0);

        // In order for us not to have to maintain a mapping of who has already
        // voted, and we don't want to count a vote twice, the signatures must
        // be submitted in strict ordering.
        for (uint idx = 0; idx < v.length; idx++){
            address signer = ecrecover(signedHash, v[idx], r[idx], s[idx]);
            require(admins[signer]);
            require(uint256(signer) > uint256(lastVoter));
            lastVoter = signer;
            emit NewCheckpointVote(_sectionIndex, _hash, v[idx], r[idx], s[idx]);

            // Sufficient signatures present, update latest checkpoint.
            if (idx+1 >= threshold){
                hash = _hash;
                height = block.number;
                sectionIndex = _sectionIndex;
                return true;
            }
        }
        // We shouldn't wind up here, reverting un-emits the events
        revert();
    }

    /**
     * @dev Get all admin addresses
     * @return address list
     */
    function GetAllAdmin()
    public
    view
    returns(address[] memory)
    {
        address[] memory ret = new address[](adminList.length);
        for (uint i = 0; i < adminList.length; i++) {
            ret[i] = adminList[i];
        }
        return ret;
    }

    /*
        Fields
    */
    // A map of admin users who have the permission to update CHT and bloom Trie root
    mapping(address => bool) admins;

    // A list of admin users so that we can obtain all admin users.
    address[] adminList;

    // Latest stored section id
    uint64 sectionIndex;

    // The block height associated with latest registered checkpoint.
    uint height;

    // The hash of latest registered checkpoint.
    bytes32 hash;

    // The frequency for creating a checkpoint
    //
    // The default value should be the same as the checkpoint size(32768) in the ethereum.
    uint sectionSize;

    // The number of confirmations needed before a checkpoint can be registered.
    // We have to make sure the checkpoint registered will not be invalid due to
    // chain reorg.
    //
    // The default value should be the same as the checkpoint process confirmations(256)
    // in the ethereum.
    uint processConfirms;

    // The required signatures to finalize a stable checkpoint.
    uint threshold;
}
@holiman
Copy link
Contributor

left a comment

This LGTM. I think it would be good if we can start playing with it in practice and maybe iron out remaining kinks as we find them. Not sure about how the deployment roadmap would look like for this one, can we merge it and have it disabled unless a certain switch (e.g. light.contract-cht) is used?

Show resolved Hide resolved cmd/puppeth/wizard_genesis.go Outdated
Show resolved Hide resolved cmd/puppeth/wizard_genesis.go Outdated
)

// newClient creates a client with specified remote URL.
func newClient(ctx *cli.Context) *ethclient.Client {

This comment has been minimized.

Copy link
@holiman

holiman May 15, 2019

Contributor

In all of these methods, you call utils.Fatalf instead of returning an error. It seems that it's generally fine, since they're only called from the exec.go, however, it's a bit deviating from the norm of returning errors and handling them.

This comment has been minimized.

Copy link
@karalabe

karalabe Jun 11, 2019

Member

Myeah, I had this debate with @fjl a while back. I wanted to replace all utils.Fatalf with log.Crit, but Felix told me that the utils version does some extra cleanup, logging, whatnot that the Crit does not, so from high level cmd things we kept using the utils.Fatal. Might be good to revisit this decision at some point, simply to unify the API.

Show resolved Hide resolved contracts/registrar/registrar.go Outdated
@rjl493456442

This comment has been minimized.

Copy link
Member Author

commented May 15, 2019

@holiman Thanks for your time! In theory we can merge this PR and disable the registrar, so that we will still use the hardcoded checkpoint, it's totally compatible. And we can also enable the registrar for some testnets to see how does it perform. Actually it won't hurt too much if there is a bug in the checkpoint contract since we can redeploy a new one(we need release a new version to change the hardcoded contract address). All old light clients will not receive new checkpoint announcements, but still works.

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch 3 times, most recently from 1a00256 to e0d2f8a May 15, 2019

@adamschmideg adamschmideg added this to the 1.9.0 milestone May 16, 2019

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch 2 times, most recently from 904f121 to 2349d77 May 20, 2019

@zsfelfoldi
Copy link
Contributor

left a comment

The new contract looks good and simple.

if (block.number < (_sectionIndex+1)*sectionSize+processConfirms) {
return false;
}
// Filter out "old" announcement

This comment has been minimized.

Copy link
@zsfelfoldi

zsfelfoldi May 24, 2019

Contributor

This logic is fine, I was just thinking about an alternative that is very simple and would even allow fixing a bad checkpoint... instead of rejecting "old" and "stale" checkpoints we could simply require _recentNumber > height. This would ensure that every signer's intention was to update the current best checkpoint.

This comment has been minimized.

Copy link
@holiman

holiman May 24, 2019

Contributor

How do you mean? What would prevent someone from submiting a stale checkpoint? The submitter can just use a new recentnumber/recenthash. Those aren't signed by the other signers, just passed by the submitter

This comment has been minimized.

Copy link
@rjl493456442

rjl493456442 May 25, 2019

Author Member

I am thinking about deleting the stale checking so that we can enable bad checkpoint fixing.

This comment has been minimized.

Copy link
@zsfelfoldi

zsfelfoldi May 31, 2019

Contributor

@holiman you are right, my mistake. It makes no sense then.

This comment has been minimized.

Copy link
@zsfelfoldi

zsfelfoldi May 31, 2019

Contributor

@rjl493456442 if we want to enable overwriting existing checkpoints then we should at least add some kind of serial number into the signed data that would prevent re-sending the previous version after the fix. Or we can leave it as it is now, the whole model assumes the reliability of the checkpoint admins anyway and I don't think we will ever set a wrong checkpoint accidentally.

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from 2349d77 to a2bec5f May 27, 2019

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch 4 times, most recently from b9107b3 to 2099bc7 Jun 1, 2019

Show resolved Hide resolved les/registrar.go Outdated
Show resolved Hide resolved params/config.go Outdated
Show resolved Hide resolved params/config.go Outdated

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from 2099bc7 to 9e4835a Jun 11, 2019

@rjl493456442

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

@karalabe Updated.

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from afdc797 to 15d322f Jun 25, 2019

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from 15d322f to f08fe75 Jun 25, 2019

Show resolved Hide resolved les/sync.go Outdated
Show resolved Hide resolved eth/config.go Outdated
Show resolved Hide resolved eth/config.go Outdated
Show resolved Hide resolved params/config.go Outdated

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from 35b60c4 to d7b6810 Jun 25, 2019

@rjl493456442

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

@karalabe Comments are addressed. PTAL.

Show resolved Hide resolved eth/backend.go Outdated
Show resolved Hide resolved params/config.go Outdated
@karalabe

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@rjl493456442

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

Fixed.

@rjl493456442 rjl493456442 force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from 694fc03 to d5fc73b Jun 26, 2019

rjl493456442 and others added some commits Sep 20, 2018

all: implement simple checkpoint syncing
cmd, les, node: remove callback mechanism

cmd, node: remove callback definition

les: simplify the registrar

les: expose checkpoint rpc services in the light client

les, light: don't store untrusted receipt

cmd, contracts, les: discard stale checkpoint

cmd, contracts/registrar: loose restriction of registeration

cmd, contracts: add replay-protection

all: off-chain multi-signature contract

params: deploy checkpoint contract for rinkeby

cmd/registrar: add raw signing mode for registrar

cmd/registrar, contracts/registrar, les: fixed messages

@karalabe karalabe force-pushed the rjl493456442:checkpoint-syncing-version-2 branch from d5fc73b to aa2697f Jun 26, 2019

Show resolved Hide resolved cmd/checkpoint-admin/main.go Outdated
Show resolved Hide resolved cmd/checkpoint-admin/main.go Outdated
Show resolved Hide resolved cmd/checkpoint-admin/exec.go Outdated
@holiman

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

Please also

diff --git a/cmd/checkpoint-admin/exec.go b/cmd/checkpoint-admin/exec.go
index 73c4fa946..3b6e81dac 100644
--- a/cmd/checkpoint-admin/exec.go
+++ b/cmd/checkpoint-admin/exec.go
@@ -25,6 +25,7 @@ import (
        "strings"
        "time"
 
+       "github.com/ethereum/go-ethereum/accounts"
        "github.com/ethereum/go-ethereum/accounts/abi/bind"
        "github.com/ethereum/go-ethereum/cmd/utils"
        "github.com/ethereum/go-ethereum/common"
@@ -222,7 +223,7 @@ func sign(ctx *cli.Context) error {
                binary.BigEndian.PutUint64(buf, cindex)
                p["address"] = address.Hex()
                p["message"] = hexutil.Encode(append(buf, chash.Bytes()...))
-               if err := clef.Call(&signature, "account_signData", "data/validator", signer, p); err != nil {
+               if err := clef.Call(&signature, "account_signData", accounts.MimetypeDataWithValidator, signer, p); err != nil {
                        utils.Fatalf("Failed to sign checkpoint, err %v", err)
                }
        case ctx.GlobalIsSet(keyFileFlag.Name):

rjl493456442 and others added some commits Jun 26, 2019

@karalabe karalabe merged commit f7cdea2 into ethereum:master Jun 28, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

t4n6a1ka added a commit to t4n6a1ka/go-ethereum that referenced this pull request Jul 1, 2019

Master (#2)
/*/
# * eth: fix sync bloom panic (ethereum#19757).

# * eth: fix sync bloom panic.

# * eth: delete useless test cases.

* mobile: fix mobile interface (ethereum#19180)

* mobile: fix mobile interface

* mobile, accounts: generate correct java binding

* accounts: fix java type binding

* mobile: support integer slice

* accounts/abi/bind, cmd/abigen: implement java binding tests

* les: prefer nil slices over zero-length slices (ethereum#19081)

* all: on-chain oracle checkpoint syncing (ethereum#19543)

* all: implement simple checkpoint syncing

cmd, les, node: remove callback mechanism

cmd, node: remove callback definition

les: simplify the registrar

les: expose checkpoint rpc services in the light client

les, light: don't store untrusted receipt

cmd, contracts, les: discard stale checkpoint

cmd, contracts/registrar: loose restriction of registeration

cmd, contracts: add replay-protection

all: off-chain multi-signature contract

params: deploy checkpoint contract for rinkeby

cmd/registrar: add raw signing mode for registrar

cmd/registrar, contracts/registrar, les: fixed messages

* cmd/registrar, contracts/registrar: fix lints

* accounts/abi/bind, les: address comments

* cmd, contracts, les, light, params: minor checkpoint sync cleanups

* cmd, eth, les, light: move checkpoint config to config file

* cmd, eth, les, params: address comments

* eth, les, params: address comments

* cmd: polish up the checkpoint admin CLI

* cmd, contracts, params: deploy new version contract

* cmd/checkpoint-admin: add another flag for clef mode signing

* cmd, contracts, les: rename and regen checkpoint oracle with abigen

* accounts/abi: Fix method overwritten by same name methods. (ethereum#17099)

* accounts/abi: Fix method overwritten by same name methods.

* accounts/abi: Fix method overwritten by same name methods.

* accounts/abi: avoid possible name conflict

Co-authored-by: Guillaume Ballet <gballet@gmail.com>

* Delete COPYING

* Create LICENSE
/*/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.