fix(coordinator): fix cannot get ProverTask correctly (#805)

Co-authored-by: HAOYUatHZ <HAOYUatHZ@users.noreply.github.com>

.github/scripts/bump_version_dot_go.mjs

@@ -0,0 +1,37 @@
+import { URL } from "url";
+import { readFileSync, writeFileSync } from "fs";
+
+const versionFilePath = new URL(
+  "../../common/version/version.go",
+  import.meta.url
+).pathname;
+
+const versionFileContent = readFileSync(versionFilePath, { encoding: "utf-8" });
+
+const currentVersion = versionFileContent.match(
+  /var tag = "(?<version>v(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+))"/
+);
+
+try {
+  parseInt(currentVersion.groups.major);
+  parseInt(currentVersion.groups.minor);
+  parseInt(currentVersion.groups.patch);
+} catch (err) {
+  console.error(new Error("Failed to parse version in version.go file"));
+  throw err;
+}
+
+// prettier-ignore
+const newVersion = `v${currentVersion.groups.major}.${currentVersion.groups.minor}.${parseInt(currentVersion.groups.patch) + 1}`;
+
+console.log(
+  `Bump version from ${currentVersion.groups.version} to ${newVersion}`
+);
+
+writeFileSync(
+  versionFilePath,
+  versionFileContent.replace(
+    `var tag = "${currentVersion.groups.version}"`,
+    `var tag = "${newVersion}"`
+  )
+);

.github/workflows/bump_version.yml

@@ -0,0 +1,55 @@
+name: Bump Version
+
+on:
+  pull_request:
+    branches: [develop]
+    types:
+      - opened
+      - reopened
+      - synchronize
+      - ready_for_review
+
+jobs:
+  try-to-bump:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+      - name: check diff
+        id: check_diff
+        run: |
+          set -euo pipefail
+
+          # fetch develop branch so that we can diff against later
+          git fetch origin develop
+
+          echo 'checking verion changes in diff...'
+
+          # check if version changed in version.go
+          # note: the grep will fail if use \d instead of [0-9]
+          git diff HEAD..origin/develop --text --no-ext-diff --unified=0 --no-prefix common/version/version.go | grep -E '^\+var tag = "v[0-9]+\.[0-9]+\.[0-9]+"$' && true
+
+          exit_code=$?
+
+          # auto bump if version is not bumped manually
+          echo '> require auto version bump?'
+
+          if [ $exit_code -eq 0 ]; then
+            echo '> no, already bumped'
+            echo "result=no-bump" >> "$GITHUB_OUTPUT"
+          else
+            echo '> yes'
+            echo "result=bump" >> "$GITHUB_OUTPUT"
+          fi
+      - name: Install Node.js 16
+        if: steps.check_diff.outputs.result == 'bump'
+        uses: actions/setup-node@v3
+        with:
+          node-version: 16
+      - name: bump version in common/version/version.go
+        if: steps.check_diff.outputs.result == 'bump'
+        run: node .github/scripts/bump_version_dot_go.mjs
+      - uses: stefanzweifel/git-auto-commit-action@3ea6ae190baf489ba007f7c92608f33ce20ef04a
+        if: steps.check_diff.outputs.result == 'bump'
+        with:
+          commit_message: "chore: auto version bump [bot]"

bridge/internal/controller/sender/sender.go

@@ -488,7 +488,7 @@ func (s *Sender) loop(ctx context.Context) {
 		case <-checkBalanceTicker.C:
 			// Check and set balance.
 			if err := s.checkBalance(ctx); err != nil {
-				log.Error("check balance, err: %w", err)
+				log.Error("check balance error", "err", err)
 			}
 		case <-ctx.Done():
 			return

common/libzkp/impl/Cargo.lock

@@ -2755,7 +2755,7 @@ dependencies = [
 [[package]]
 name = "prover"
 version = "0.4.0"
-source = "git+https://github.com/scroll-tech/scroll-prover?tag=v0.5.16#bd901762c4744936586f4a30e27a4b65cea3bb71"
+source = "git+https://github.com/scroll-tech/scroll-prover?tag=v0.5.18#fcf4bf3137ad37becdeb5360b23ac978405c6b2c"
 dependencies = [
  "aggregator",
  "anyhow",
@@ -4040,7 +4040,7 @@ checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba"
 [[package]]
 name = "types"
 version = "0.4.0"
-source = "git+https://github.com/scroll-tech/scroll-prover?tag=v0.5.16#bd901762c4744936586f4a30e27a4b65cea3bb71"
+source = "git+https://github.com/scroll-tech/scroll-prover?tag=v0.5.18#fcf4bf3137ad37becdeb5360b23ac978405c6b2c"
 dependencies = [
  "base64 0.13.1",
  "blake2",

common/libzkp/impl/Cargo.toml

@@ -20,8 +20,8 @@ maingate = { git = "https://github.com/scroll-tech/halo2wrong", branch = "halo2-
 halo2curves = { git = "https://github.com/scroll-tech/halo2curves.git", branch = "0.3.1-derive-serde" }
 
 [dependencies]
-prover = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.5.16" }
-types = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.5.16" }
+prover = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.5.18" }
+types = { git = "https://github.com/scroll-tech/scroll-prover", tag = "v0.5.18" }
 halo2_proofs = { git = "https://github.com/scroll-tech/halo2.git", branch = "develop" }
 
 log = "0.4"

common/types/message/message.go

@@ -250,12 +250,14 @@ type ChunkInfo struct {
 
 // ChunkProof includes the proof info that are required for chunk verification and rollup.
 type ChunkProof struct {
-	StorageTrace []byte     `json:"storage_trace"`
-	Protocol     []byte     `json:"protocol"`
-	Proof        []byte     `json:"proof"`
-	Instances    []byte     `json:"instances"`
-	Vk           []byte     `json:"vk"`
-	ChunkInfo    *ChunkInfo `json:"chunk_info,omitempty"` // cross-reference between cooridinator computation and prover compution
+	StorageTrace []byte `json:"storage_trace"`
+	Protocol     []byte `json:"protocol"`
+	Proof        []byte `json:"proof"`
+	Instances    []byte `json:"instances"`
+	Vk           []byte `json:"vk"`
+	// cross-reference between cooridinator computation and prover compution
+	ChunkInfo  *ChunkInfo `json:"chunk_info,omitempty"`
+	GitVersion string     `json:"git_version,omitempty"`
 }
 
 // BatchProof includes the proof info that are required for batch verification and rollup.
@@ -263,6 +265,8 @@ type BatchProof struct {
 	Proof     []byte `json:"proof"`
 	Instances []byte `json:"instances"`
 	Vk        []byte `json:"vk"`
+	// cross-reference between cooridinator computation and prover compution
+	GitVersion string `json:"git_version,omitempty"`
 }
 
 // SanityCheck checks whether an BatchProof is in a legal format

common/types/message/message_test.go

@@ -108,7 +108,7 @@ func TestProofDetailHash(t *testing.T) {
 	}
 	hash, err := proofDetail.Hash()
 	assert.NoError(t, err)
-	expectedHash := "72a00232c1fcb100b1b67e6d12cd449e5d2d890e3a66e50f4c23499d4990766f"
+	expectedHash := "d3b57cb84b0da8043373eeb3612806fb7248d6d1b6e089846ccf3ccce2d9f31c"
 	assert.Equal(t, expectedHash, hex.EncodeToString(hash))
 }
 

common/version/version.go

@@ -6,7 +6,7 @@ import (
 	"strings"
 )
 
-var tag = "v4.1.46"
+var tag = "v4.1.55"
 
 var commit = func() string {
 	if info, ok := debug.ReadBuildInfo(); ok {

contracts/src/misc/ScrollOwner.sol

@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import {AccessControlEnumerable} from "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
+import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
+
+// solhint-disable no-empty-blocks
+
+contract ScrollOwner is AccessControlEnumerable {
+    using EnumerableSet for EnumerableSet.Bytes32Set;
+
+    /*************
+     * Variables *
+     *************/
+
+    /// @notice Mapping from target address to selector to the list of accessible roles.
+    mapping(address => mapping(bytes4 => EnumerableSet.Bytes32Set)) private targetAccess;
+
+    /**********************
+     * Function Modifiers *
+     **********************/
+
+    modifier hasAccess(
+        address _target,
+        bytes4 _selector,
+        bytes32 _role
+    ) {
+        // admin has access to all methods
+        require(_role == DEFAULT_ADMIN_ROLE || targetAccess[_target][_selector].contains(_role), "no access");
+        _;
+    }
+
+    /***************
+     * Constructor *
+     ***************/
+
+    constructor() {
+        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
+    }
+
+    /*************************
+     * Public View Functions *
+     *************************/
+
+    /// @notice Return a list of roles which has access to the function.
+    /// @param _target The address of target contract.
+    /// @param _selector The function selector to query.
+    /// @return _roles The list of roles.
+    function callableRoles(address _target, bytes4 _selector) external view returns (bytes32[] memory _roles) {
+        EnumerableSet.Bytes32Set storage _lists = targetAccess[_target][_selector];
+        _roles = new bytes32[](_lists.length());
+        for (uint256 i = 0; i < _roles.length; i++) {
+            _roles[i] = _lists.at(i);
+        }
+    }
+
+    /*****************************
+     * Public Mutating Functions *
+     *****************************/
+
+    /// @notice Perform a function call from arbitrary role.
+    /// @param _target The address of target contract.
+    /// @param _value The value passing to target contract.
+    /// @param _data The calldata passing to target contract.
+    /// @param _role The expected role of the caller.
+    function execute(
+        address _target,
+        uint256 _value,
+        bytes calldata _data,
+        bytes32 _role
+    ) public payable onlyRole(_role) hasAccess(_target, bytes4(_data[0:4]), _role) {
+        _execute(_target, _value, _data);
+    }
+
+    // allow others to send ether to this contract.
+    receive() external payable {}
+
+    /************************
+     * Restricted Functions *
+     ************************/
+
+    /// @notice Update the access to target contract.
+    /// @param _target The address of target contract.
+    /// @param _selectors The list of function selectors to update.
+    /// @param _role The role to change.
+    /// @param _status True if we are going to add the role, otherwise remove the role.
+    function updateAccess(
+        address _target,
+        bytes4[] memory _selectors,
+        bytes32 _role,
+        bool _status
+    ) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        if (_status) {
+            for (uint256 i = 0; i < _selectors.length; i++) {
+                targetAccess[_target][_selectors[i]].add(_role);
+            }
+        } else {
+            for (uint256 i = 0; i < _selectors.length; i++) {
+                targetAccess[_target][_selectors[i]].remove(_role);
+            }
+        }
+    }
+
+    /**********************
+     * Internal Functions *
+     **********************/
+
+    /// @dev Internal function to call contract. If the call reverted, the error will be popped up.
+    /// @param _target The address of target contract.
+    /// @param _value The value passing to target contract.
+    /// @param _data The calldata passing to target contract.
+    function _execute(
+        address _target,
+        uint256 _value,
+        bytes calldata _data
+    ) internal {
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, ) = address(_target).call{value: _value}(_data);
+        if (!success) {
+            // solhint-disable-next-line no-inline-assembly
+            assembly {
+                let ptr := mload(0x40)
+                let size := returndatasize()
+                returndatacopy(ptr, 0, size)
+                revert(ptr, size)
+            }
+        }
+    }
+}

contracts/src/test/ScrollOwner.t.sol

@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import {DSTestPlus} from "solmate/test/utils/DSTestPlus.sol";
+
+import {ScrollOwner} from "../misc/ScrollOwner.sol";
+
+contract ScrollOwnerTest is DSTestPlus {
+    event Call();
+
+    ScrollOwner private owner;
+
+    function setUp() public {
+        owner = new ScrollOwner();
+    }
+
+    function testUpdateAccess() external {
+        // not admin, evert
+        hevm.startPrank(address(1));
+        hevm.expectRevert(
+            "AccessControl: account 0x0000000000000000000000000000000000000001 is missing role 0x0000000000000000000000000000000000000000000000000000000000000000"
+        );
+        owner.updateAccess(address(0), new bytes4[](0), bytes32(0), true);
+        hevm.stopPrank();
+
+        bytes4[] memory _selectors;
+        bytes32[] memory _roles;
+
+        // add access then remove access
+        _roles = owner.callableRoles(address(this), ScrollOwnerTest.revertOnCall.selector);
+        assertEq(0, _roles.length);
+        _selectors = new bytes4[](1);
+        _selectors[0] = ScrollOwnerTest.revertOnCall.selector;
+        owner.updateAccess(address(this), _selectors, bytes32(uint256(1)), true);
+        _roles = owner.callableRoles(address(this), ScrollOwnerTest.revertOnCall.selector);
+        assertEq(1, _roles.length);
+        assertEq(_roles[0], bytes32(uint256(1)));
+        owner.updateAccess(address(this), _selectors, bytes32(uint256(1)), false);
+        _roles = owner.callableRoles(address(this), ScrollOwnerTest.revertOnCall.selector);
+        assertEq(0, _roles.length);
+    }
+
+    function testAdminExecute() external {
+        // call with revert
+        hevm.expectRevert("Called");
+        owner.execute(address(this), 0, abi.encodeWithSelector(ScrollOwnerTest.revertOnCall.selector), bytes32(0));
+
+        // call with emit
+        hevm.expectEmit(false, false, false, true);
+        emit Call();
+        owner.execute(address(this), 0, abi.encodeWithSelector(ScrollOwnerTest.emitOnCall.selector), bytes32(0));
+    }
+
+    function testExecute(bytes32 _role) external {
+        hevm.assume(_role != bytes32(0));
+
+        bytes4[] memory _selectors = new bytes4[](2);
+        _selectors[0] = ScrollOwnerTest.revertOnCall.selector;
+        _selectors[1] = ScrollOwnerTest.emitOnCall.selector;
+
+        owner.grantRole(_role, address(this));
+
+        // no access, revert
+        hevm.expectRevert("no access");
+        owner.execute(address(this), 0, abi.encodeWithSelector(ScrollOwnerTest.revertOnCall.selector), _role);
+
+        owner.updateAccess(address(this), _selectors, _role, true);
+
+        // call with revert
+        hevm.expectRevert("Called");
+        owner.execute(address(this), 0, abi.encodeWithSelector(ScrollOwnerTest.revertOnCall.selector), _role);
+
+        // call with emit
+        hevm.expectEmit(false, false, false, true);
+        emit Call();
+        owner.execute(address(this), 0, abi.encodeWithSelector(ScrollOwnerTest.emitOnCall.selector), _role);
+    }
+
+    function revertOnCall() external pure {
+        revert("Called");
+    }
+
+    function emitOnCall() external {
+        emit Call();
+    }
+}

coordinator/cmd/app/app.go

@@ -71,6 +71,11 @@ func action(ctx *cli.Context) error {
 
 	apiSrv := apiServer(ctx, cfg, db, registry)
 
+	log.Info(
+		"coordinator start successfully",
+		"version", version.Version,
+	)
+
 	// Catch CTRL-C to ensure a graceful shutdown.
 	interrupt := make(chan os.Signal, 1)
 	signal.Notify(interrupt, os.Interrupt)

coordinator/internal/logic/submitproof/proof_receiver.go

@@ -127,8 +127,12 @@ func (m *ProofReceiverLogic) HandleZkProof(ctx *gin.Context, proofMsg *message.P
 	if len(pk) == 0 {
 		return fmt.Errorf("get public key from context failed")
 	}
+	pv := ctx.GetString(coordinatorType.ProverVersion)
+	if len(pk) == 0 {
+		return fmt.Errorf("get ProverVersion from context failed")
+	}
 
-	proverTask, err := m.proverTaskOrm.GetProverTaskByTaskIDAndPubKey(ctx, proofMsg.ID, pk)
+	proverTask, err := m.proverTaskOrm.GetProverTaskByTaskIDAndProver(ctx, proofMsg.ID, pk, pv)
 	if proverTask == nil || err != nil {
 		log.Error("get none prover task for the proof", "key", pk, "taskID", proofMsg.ID, "error", err)
 		return ErrValidatorFailureProverTaskEmpty

coordinator/internal/orm/prover_task.go

@@ -113,16 +113,18 @@ func (o *ProverTask) GetProverTasksByHashes(ctx context.Context, hashes []string
 	return proverTasks, nil
 }
 
-// GetProverTaskByTaskIDAndPubKey get prover task taskID and public key
-func (o *ProverTask) GetProverTaskByTaskIDAndPubKey(ctx context.Context, taskID, proverPublicKey string) (*ProverTask, error) {
+// GetProverTaskByTaskIDAndProver get prover task taskID and public key
+func (o *ProverTask) GetProverTaskByTaskIDAndProver(ctx context.Context, taskID, proverPublicKey, proverVersion string) (*ProverTask, error) {
 	db := o.db.WithContext(ctx)
 	db = db.Model(&ProverTask{})
-	db = db.Where("task_id", taskID).Where("prover_public_key", proverPublicKey)
+	db = db.Where("task_id", taskID)
+	db = db.Where("prover_public_key", proverPublicKey)
+	db = db.Where("prover_version", proverVersion)
 
 	var proverTask ProverTask
 	err := db.First(&proverTask).Error
 	if err != nil {
-		return nil, fmt.Errorf("ProverTask.GetProverTaskByTaskIDAndPubKey err:%w, taskID:%s, pubukey:%s", err, taskID, proverPublicKey)
+		return nil, fmt.Errorf("ProverTask.GetProverTaskByTaskIDAndProver err:%w, taskID:%s, pubkey:%s, prover_version:%s", err, taskID, proverPublicKey, proverVersion)
 	}
 	return &proverTask, nil
 }
@@ -166,8 +168,8 @@ func (o *ProverTask) SetProverTask(ctx context.Context, proverTask *ProverTask,
 
 	db = db.Model(&ProverTask{})
 	db = db.Clauses(clause.OnConflict{
-		Columns:   []clause.Column{{Name: "task_type"}, {Name: "task_id"}, {Name: "prover_public_key"}},
-		DoUpdates: clause.AssignmentColumns([]string{"prover_version", "proving_status", "failure_type", "assigned_at"}),
+		Columns:   []clause.Column{{Name: "task_type"}, {Name: "task_id"}, {Name: "prover_public_key"}, {Name: "prover_version"}},
+		DoUpdates: clause.AssignmentColumns([]string{"proving_status", "failure_type", "assigned_at"}),
 	})
 
 	if err := db.Create(&proverTask).Error; err != nil {

database/migrate/migrations/00006_prover_task.sql

@@ -26,7 +26,7 @@ create table prover_task
     updated_at          TIMESTAMP(0)   NOT NULL DEFAULT CURRENT_TIMESTAMP,
     deleted_at          TIMESTAMP(0)   DEFAULT NULL,
 
-    CONSTRAINT uk_tasktype_taskid_publickey UNIQUE (task_type, task_id, prover_public_key)
+    CONSTRAINT uk_tasktype_taskid_publickey_version UNIQUE (task_type, task_id, prover_public_key, prover_version)
 );
 
 comment

prover-stats-api/internal/orm/prover_task.go

@@ -94,8 +94,8 @@ func (o *ProverTask) SetProverTask(ctx context.Context, proverTask *ProverTask,
 
 	db = db.Model(&ProverTask{})
 	db = db.Clauses(clause.OnConflict{
-		Columns:   []clause.Column{{Name: "task_type"}, {Name: "task_id"}, {Name: "prover_public_key"}},
-		DoUpdates: clause.AssignmentColumns([]string{"prover_version", "proving_status", "failure_type", "assigned_at"}),
+		Columns:   []clause.Column{{Name: "task_type"}, {Name: "task_id"}, {Name: "prover_public_key"}, {Name: "prover_version"}},
+		DoUpdates: clause.AssignmentColumns([]string{"proving_status", "failure_type", "assigned_at"}),
 	})
 
 	if err := db.Create(&proverTask).Error; err != nil {

prover/cmd/app/app.go

@@ -52,9 +52,11 @@ func action(ctx *cli.Context) error {
 	r.Start()
 
 	defer r.Stop()
-	log.Info("prover start successfully",
+	log.Info(
+		"prover start successfully",
 		"name", cfg.ProverName, "type", cfg.Core.ProofType,
-		"publickey", r.PublicKey(), "version", version.Version)
+		"publickey", r.PublicKey(), "version", version.Version,
+	)
 
 	// Catch CTRL-C to ensure a graceful shutdown.
 	interrupt := make(chan os.Signal, 1)