From 250033a4e2972c48064c694759bb674eeb72f6d3 Mon Sep 17 00:00:00 2001 From: jeff <113397187+cyberhorsey@users.noreply.github.com> Date: Fri, 8 Mar 2024 12:07:51 -0800 Subject: [PATCH 1/6] chore(eventindexer): update swagger documentation + cache key (#16357) --- packages/eventindexer/docs/docs.go | 26 ++++++++++++++++--- packages/eventindexer/docs/swagger.json | 22 +++++++++++++++- packages/eventindexer/docs/swagger.yaml | 16 +++++++++++- .../pkg/http/get_chart_by_task.go | 13 +++++++--- 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/packages/eventindexer/docs/docs.go b/packages/eventindexer/docs/docs.go index 6babd71d1c..65a6411b2c 100644 --- a/packages/eventindexer/docs/docs.go +++ b/packages/eventindexer/docs/docs.go @@ -1,4 +1,4 @@ -// Package docs Code generated by swaggo/swag. DO NOT EDIT +// Code generated by swaggo/swag. DO NOT EDIT package docs import "github.com/swaggo/swag" @@ -52,7 +52,7 @@ const docTemplate = `{ } } }, - "/chartByTask": { + "/chart/chartByTask": { "get": { "consumes": [ "application/json" @@ -69,6 +69,20 @@ const docTemplate = `{ "name": "task", "in": "query", "required": true + }, + { + "type": "string", + "description": "start date", + "name": "start", + "in": "query", + "required": true + }, + { + "type": "string", + "description": "end date", + "name": "end", + "in": "query", + "required": true } ], "responses": { @@ -316,6 +330,12 @@ const docTemplate = `{ "paginate.Page": { "type": "object", "properties": { + "error": { + "type": "boolean" + }, + "error_message": { + "type": "string" + }, "first": { "type": "boolean" }, @@ -356,8 +376,6 @@ var SwaggerInfo = &swag.Spec{ Description: "", InfoInstanceName: "swagger", SwaggerTemplate: docTemplate, - LeftDelim: "{{", - RightDelim: "}}", } func init() { diff --git a/packages/eventindexer/docs/swagger.json b/packages/eventindexer/docs/swagger.json index 0e8b34a8e1..c693053f63 100644 --- a/packages/eventindexer/docs/swagger.json +++ b/packages/eventindexer/docs/swagger.json @@ -40,7 +40,7 @@ } } }, - "/chartByTask": { + "/chart/chartByTask": { "get": { "consumes": ["application/json"], "produces": ["application/json"], @@ -53,6 +53,20 @@ "name": "task", "in": "query", "required": true + }, + { + "type": "string", + "description": "start date", + "name": "start", + "in": "query", + "required": true + }, + { + "type": "string", + "description": "end date", + "name": "end", + "in": "query", + "required": true } ], "responses": { @@ -280,6 +294,12 @@ "paginate.Page": { "type": "object", "properties": { + "error": { + "type": "boolean" + }, + "error_message": { + "type": "string" + }, "first": { "type": "boolean" }, diff --git a/packages/eventindexer/docs/swagger.yaml b/packages/eventindexer/docs/swagger.yaml index 0afa8819b9..877a55c026 100644 --- a/packages/eventindexer/docs/swagger.yaml +++ b/packages/eventindexer/docs/swagger.yaml @@ -64,6 +64,10 @@ definitions: type: object paginate.Page: properties: + error: + type: boolean + error_message: + type: string first: type: boolean items: {} @@ -113,7 +117,7 @@ paths: schema: $ref: "#/definitions/paginate.Page" summary: Get assigned blocks by prover address - /chartByTask: + /chart/chartByTask: get: consumes: - application/json @@ -124,6 +128,16 @@ paths: name: task required: true type: string + - description: start date + in: query + name: start + required: true + type: string + - description: end date + in: query + name: end + required: true + type: string produces: - application/json responses: diff --git a/packages/eventindexer/pkg/http/get_chart_by_task.go b/packages/eventindexer/pkg/http/get_chart_by_task.go index 370cb3db63..a4b25d6d31 100644 --- a/packages/eventindexer/pkg/http/get_chart_by_task.go +++ b/packages/eventindexer/pkg/http/get_chart_by_task.go @@ -16,12 +16,19 @@ import ( // @Summary Get time series data for displaying charts // @ID get-charts-by-task // @Param task query string true "task to query" +// @Param start query string true "start date" +// @Param end query string true "end date" // @Accept json // @Produce json // @Success 200 {object} eventindexer.ChartResponse -// @Router /chartByTask [get] +// @Router /chart/chartByTask [get] func (srv *Server) GetChartByTask(c echo.Context) error { - cached, found := srv.cache.Get(c.QueryParam("task") + c.QueryParam("fee_token_address") + c.QueryParam("tier")) + cacheKey := c.QueryParam("task") + + c.QueryParam("fee_token_address") + + c.QueryParam("tier") + + c.QueryParam("start") + + c.QueryParam("end") + cached, found := srv.cache.Get(cacheKey) var chart *eventindexer.ChartResponse @@ -43,7 +50,7 @@ func (srv *Server) GetChartByTask(c echo.Context) error { } srv.cache.Set( - c.QueryParam("task")+c.QueryParam("fee_token_address")+c.QueryParam("tier"), + cacheKey, chart, cache.DefaultExpiration, ) From 9eafbd0c5ef3cf03656cb8b7e7581b7100108901 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 9 Mar 2024 03:41:17 +0000 Subject: [PATCH 2/6] chore(deps-dev): bump autoprefixer from 10.4.17 to 10.4.18 (#16244) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: d1onys1us <13951458+d1onys1us@users.noreply.github.com> --- packages/bridge-ui/package.json | 2 +- .../package.json | 2 +- pnpm-lock.yaml | 27 ++++++++----------- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/bridge-ui/package.json b/packages/bridge-ui/package.json index 5bee87cdf9..c1ddceaee8 100644 --- a/packages/bridge-ui/package.json +++ b/packages/bridge-ui/package.json @@ -33,7 +33,7 @@ "@wagmi/cli": "^2.1.1", "abitype": "^1.0.0", "ajv": "^8.12.0", - "autoprefixer": "^10.4.17", + "autoprefixer": "^10.4.18", "daisyui": "^4.7.2", "dotenv": "^16.4.5", "eslint": "^8.56.0", diff --git a/packages/guardian-prover-health-check-ui/package.json b/packages/guardian-prover-health-check-ui/package.json index 4b3be9c5a1..aa792f6bec 100644 --- a/packages/guardian-prover-health-check-ui/package.json +++ b/packages/guardian-prover-health-check-ui/package.json @@ -23,7 +23,7 @@ "@tailwindcss/nesting": "0.0.0-insiders.565cd3e", "@typescript-eslint/eslint-plugin": "^7.1.0", "@typescript-eslint/parser": "^7.0.2", - "autoprefixer": "^10.4.17", + "autoprefixer": "^10.4.18", "daisyui": "^4.7.2", "eslint": "^8.56.0", "eslint-config-prettier": "^9.1.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ae9f9e1f6c..1a12676406 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -96,8 +96,8 @@ importers: specifier: ^8.12.0 version: 8.12.0 autoprefixer: - specifier: ^10.4.17 - version: 10.4.17(postcss@8.4.35) + specifier: ^10.4.18 + version: 10.4.18(postcss@8.4.35) daisyui: specifier: ^4.7.2 version: 4.7.2(postcss@8.4.35) @@ -208,8 +208,8 @@ importers: specifier: ^7.0.2 version: 7.0.2(eslint@8.56.0)(typescript@5.3.3) autoprefixer: - specifier: ^10.4.17 - version: 10.4.17(postcss@8.4.35) + specifier: ^10.4.18 + version: 10.4.18(postcss@8.4.35) daisyui: specifier: ^4.7.2 version: 4.7.2(postcss@8.4.35) @@ -6516,15 +6516,15 @@ packages: engines: {node: '>=8.0.0'} dev: false - /autoprefixer@10.4.17(postcss@8.4.35): - resolution: {integrity: sha512-/cpVNRLSfhOtcGflT13P2794gVSgmPgTR+erw5ifnMLZb0UnSlkK4tquLmkd3BhA+nLo5tX8Cu0upUsGKvKbmg==} + /autoprefixer@10.4.18(postcss@8.4.35): + resolution: {integrity: sha512-1DKbDfsr6KUElM6wg+0zRNkB/Q7WcKYAaK+pzXn+Xqmszm/5Xa9coeNdtP88Vi+dPzZnMjhge8GIV49ZQkDa+g==} engines: {node: ^10 || ^12 || >=14} hasBin: true peerDependencies: postcss: ^8.1.0 dependencies: browserslist: 4.23.0 - caniuse-lite: 1.0.30001588 + caniuse-lite: 1.0.30001593 fraction.js: 4.3.7 normalize-range: 0.1.2 picocolors: 1.0.0 @@ -6730,7 +6730,7 @@ packages: engines: {node: ^6 || ^7 || ^8 || ^9 || ^10 || ^11 || ^12 || >=13.7} hasBin: true dependencies: - caniuse-lite: 1.0.30001588 + caniuse-lite: 1.0.30001593 electron-to-chromium: 1.4.676 node-releases: 2.0.14 update-browserslist-db: 1.0.13(browserslist@4.23.0) @@ -6898,8 +6898,8 @@ packages: engines: {node: '>=10'} dev: false - /caniuse-lite@1.0.30001588: - resolution: {integrity: sha512-+hVY9jE44uKLkH0SrUTqxjxqNTOWHsbnQDIKjwkZ3lNTzUUVdBLBGXtj/q5Mp5u98r3droaZAewQuEDzjQdZlQ==} + /caniuse-lite@1.0.30001593: + resolution: {integrity: sha512-UWM1zlo3cZfkpBysd7AS+z+v007q9G1+fLTUU42rQnY6t2axoogPW/xol6T7juU5EUoOhML4WgBIdG+9yYqAjQ==} /capital-case@1.0.4: resolution: {integrity: sha512-ds37W8CytHgwnhGGTi88pcPyR15qoNkOpYwmMMfnWqqWgESapLqvDx6huFjQ5vqWSn2Z06173XNA7LtMOeUh1A==} @@ -8004,14 +8004,9 @@ packages: '@esbuild/win32-ia32': 0.19.12 '@esbuild/win32-x64': 0.19.12 - /escalade@3.1.1: - resolution: {integrity: sha512-k0er2gUkLf8O0zKJiAhmkTnJlTvINGv7ygDNPbeIsX/TJjGJZHuh9B2UxbsaEkmlEo9MfhrSzmhIlhRlI2GXnw==} - engines: {node: '>=6'} - /escalade@3.1.2: resolution: {integrity: sha512-ErCHMCae19vR8vQGe50xIsVomy19rg6gFu3+r3jkEO46suLMWBksvVyoGgQV+jOfl84ZSOSlmv6Gxa89PmTGmA==} engines: {node: '>=6'} - dev: false /escape-html@1.0.3: resolution: {integrity: sha512-NiSupZ4OeuGwr68lGIeym/ksIZMJodUGOSCZ/FSnTxcrekbvqrgdUxlJOMpijaKZVjAJrWrGs/6Jy8OMuyj9ow==} @@ -13878,7 +13873,7 @@ packages: browserslist: '>= 4.21.0' dependencies: browserslist: 4.23.0 - escalade: 3.1.1 + escalade: 3.1.2 picocolors: 1.0.0 /upper-case-first@2.0.2: From 5c3f2b27de2db38b98288e7b01ccc09b00ec1d73 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 9 Mar 2024 03:41:39 +0000 Subject: [PATCH 3/6] chore(deps): bump github.com/stretchr/testify from 1.8.4 to 1.9.0 (#16241) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: d1onys1us <13951458+d1onys1us@users.noreply.github.com> --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 0c7812de14..296dadc43c 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/prometheus/client_golang v1.19.0 github.com/rabbitmq/amqp091-go v1.9.0 github.com/shopspring/decimal v1.3.1 - github.com/stretchr/testify v1.8.4 + github.com/stretchr/testify v1.9.0 github.com/swaggo/swag v1.16.3 github.com/testcontainers/testcontainers-go v0.28.0 github.com/urfave/cli/v2 v2.27.1 diff --git a/go.sum b/go.sum index cca17e718c..15e99315ea 100644 --- a/go.sum +++ b/go.sum @@ -611,8 +611,9 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/supranational/blst v0.3.11 h1:LyU6FolezeWAhvQk0k6O/d49jqgO52MSDDfYgbeoEm4= github.com/supranational/blst v0.3.11/go.mod h1:jZJtfjgudtNl4en1tzwPIV3KjUnQUvG3/j+w+fVonLw= github.com/swaggo/swag v1.16.3 h1:PnCYjPCah8FK4I26l2F/KQ4yz3sILcVUN3cTlBFA9Pg= From 2a0fe9526718bdf799874c7f2b0968f3dda7b6f2 Mon Sep 17 00:00:00 2001 From: Daniel Wang <99078276+dantaik@users.noreply.github.com> Date: Sat, 9 Mar 2024 11:45:13 +0800 Subject: [PATCH 4/6] feat(protocol): upgrade to use OZ 4.9.6 (#16360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: D <51912515+adaki2004@users.noreply.github.com> Co-authored-by: Keszey Dániel Co-authored-by: d1onys1us <13951458+d1onys1us@users.noreply.github.com> --- .../contracts/L1/gov/TaikoGovernor.sol | 54 +++++++------------ packages/protocol/package.json | 4 +- .../protocol/test/L1/gov/TaikoGovernor.t.sol | 5 -- pnpm-lock.yaml | 32 ++++++++--- 4 files changed, 46 insertions(+), 49 deletions(-) diff --git a/packages/protocol/contracts/L1/gov/TaikoGovernor.sol b/packages/protocol/contracts/L1/gov/TaikoGovernor.sol index 6a44820ee5..6fbfe9d6d5 100644 --- a/packages/protocol/contracts/L1/gov/TaikoGovernor.sol +++ b/packages/protocol/contracts/L1/gov/TaikoGovernor.sol @@ -1,22 +1,19 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; -import "@openzeppelin/contracts-upgradeable/governance/GovernorUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/governance/compatibility/GovernorCompatibilityBravoUpgradeable.sol"; -import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorVotesQuorumFractionUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/governance/extensions/GovernorTimelockControlUpgradeable.sol"; -import "../../common/EssentialContract.sol"; +import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol"; /// @title TaikoGovernor /// @custom:security-contact security@taiko.xyz contract TaikoGovernor is - EssentialContract, + Ownable2StepUpgradeable, GovernorCompatibilityBravoUpgradeable, - GovernorVotesUpgradeable, GovernorVotesQuorumFractionUpgradeable, GovernorTimelockControlUpgradeable { @@ -36,9 +33,8 @@ contract TaikoGovernor is external initializer { - __Essential_init(_owner); + _transferOwnership(_owner == address(0) ? msg.sender : _owner); __Governor_init("TaikoGovernor"); - __GovernorCompatibilityBravo_init(); __GovernorVotes_init(_token); __GovernorVotesQuorumFraction_init(4); __GovernorTimelockControl_init(_timelock); @@ -58,33 +54,6 @@ contract TaikoGovernor is return super.propose(_targets, _values, _calldatas, _description); } - /// @notice An overwrite of GovernorCompatibilityBravoUpgradeable's propose() as that one does - /// not check that the length of signatures equal the calldata. - /// @dev See vulnerability description here: - /// https://github.com/taikoxyz/taiko-mono/security/dependabot/114 - /// See fix in OZ 4.8.3 here (URL broken down for readability): - /// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/ - /// 0a25c1940ca220686588c4af3ec526f725fe2582/contracts/governance/compatibility/GovernorCompatibilityBravo.sol#L72 - /// See {GovernorCompatibilityBravoUpgradeable-propose} - function propose( - address[] memory _targets, - uint256[] memory _values, - string[] memory _signatures, - bytes[] memory _calldatas, - string memory _description - ) - public - virtual - override(GovernorCompatibilityBravoUpgradeable) - returns (uint256) - { - if (_signatures.length != _calldatas.length) revert TG_INVALID_SIGNATURES_LENGTH(); - - return GovernorCompatibilityBravoUpgradeable.propose( - _targets, _values, _signatures, _calldatas, _description - ); - } - /// @dev See {GovernorUpgradeable-supportsInterface} function supportsInterface(bytes4 _interfaceId) public @@ -124,6 +93,23 @@ contract TaikoGovernor is return 1_000_000_000 ether / 10_000; // 0.01% of Taiko Token } + /// @dev Cancel a proposal with GovernorBravo logic. + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) + public + virtual + override(IGovernorUpgradeable, GovernorUpgradeable, GovernorCompatibilityBravoUpgradeable) + returns (uint256) + { + return GovernorCompatibilityBravoUpgradeable.cancel( + targets, values, calldatas, descriptionHash + ); + } + function _execute( uint256 _proposalId, address[] memory _targets, diff --git a/packages/protocol/package.json b/packages/protocol/package.json index d96f493788..280d85d118 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -35,8 +35,8 @@ "typescript": "^5.2.2" }, "dependencies": { - "@openzeppelin/contracts": "4.8.2", - "@openzeppelin/contracts-upgradeable": "4.8.2", + "@openzeppelin/contracts": "4.9.6", + "@openzeppelin/contracts-upgradeable": "4.9.6", "ds-test": "github:dapphub/ds-test#e282159d5170298eb2455a6c05280ab5a73a4ef0", "forge-std": "github:foundry-rs/forge-std#v1.7.5", "merkletreejs": "^0.3.11", diff --git a/packages/protocol/test/L1/gov/TaikoGovernor.t.sol b/packages/protocol/test/L1/gov/TaikoGovernor.t.sol index 76fb7b6c86..e9a42b57de 100644 --- a/packages/protocol/test/L1/gov/TaikoGovernor.t.sol +++ b/packages/protocol/test/L1/gov/TaikoGovernor.t.sol @@ -99,11 +99,6 @@ contract TestTaikoGovernor is TaikoL1TestBase { true, "Incorrect supports interface" ); - assertEq( - taikoGovernor.supportsInterface(type(IGovernorUpgradeable).interfaceId), - true, - "Incorrect supports interface" - ); assertEq( taikoGovernor.supportsInterface(type(IERC1155ReceiverUpgradeable).interfaceId), true, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1a12676406..ba3fdd7b4f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -265,11 +265,11 @@ importers: packages/protocol: dependencies: '@openzeppelin/contracts': - specifier: 4.8.2 - version: 4.8.2 + specifier: 4.9.6 + version: 4.9.6 '@openzeppelin/contracts-upgradeable': - specifier: 4.8.2 - version: 4.8.2 + specifier: 4.9.6 + version: 4.9.6 ds-test: specifier: github:dapphub/ds-test#e282159d5170298eb2455a6c05280ab5a73a4ef0 version: github.com/dapphub/ds-test/e282159d5170298eb2455a6c05280ab5a73a4ef0 @@ -3301,12 +3301,12 @@ packages: fastq: 1.17.1 dev: true - /@openzeppelin/contracts-upgradeable@4.8.2: - resolution: {integrity: sha512-zIggnBwemUmmt9IS73qxi+tumALxCY4QEs3zLCII78k0Gfse2hAOdAkuAeLUzvWUpneMUfFE5sGHzEUSTvn4Ag==} + /@openzeppelin/contracts-upgradeable@4.9.6: + resolution: {integrity: sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA==} dev: false - /@openzeppelin/contracts@4.8.2: - resolution: {integrity: sha512-kEUOgPQszC0fSYWpbh2kT94ltOJwj1qfT2DWo+zVttmGmf97JZ99LspePNaeeaLhCImaHVeBbjaQFZQn7+Zc5g==} + /@openzeppelin/contracts@4.9.6: + resolution: {integrity: sha512-xSmezSupL+y9VkHZJGDoCBpmnB2ogM13ccaYDWqJTfS3dbuHkgjuwDFUmaFauBCboQMGB/S5UqUl2y54X99BmA==} dev: false /@parcel/watcher-android-arm64@2.4.0: @@ -6100,6 +6100,13 @@ packages: /@web3modal/siwe@4.0.9(typescript@5.3.3): resolution: {integrity: sha512-OB4z/lTHCAm3bjiuyPz4uBib46YU6kzp4eeSnAWZzAHj9mQnB4DZOoCdFQvFn+N1n3CzTZaMxz3CYjYn2A+Qhw==} requiresBuild: true + peerDependenciesMeta: + react: + optional: true + react-dom: + optional: true + vue: + optional: true dependencies: '@web3modal/core': 4.0.9(react@18.2.0) '@web3modal/scaffold-utils': 4.0.9(react@18.2.0) @@ -6126,6 +6133,15 @@ packages: '@wagmi/connectors': '>=4.0.0' '@wagmi/core': '>=2.0.0' viem: '>=2.0.0' + peerDependenciesMeta: + '@web3modal/siwe': + optional: true + react: + optional: true + react-dom: + optional: true + vue: + optional: true dependencies: '@wagmi/connectors': 4.1.14(@wagmi/core@2.6.5)(react-dom@18.2.0)(react-native@0.73.4)(react@18.2.0)(typescript@5.3.3)(viem@2.7.11) '@wagmi/core': 2.6.5(react@18.2.0)(typescript@5.3.3)(viem@2.7.11) From 0b1385e37b9d17fb9ce41fcb48793fc2b8fc468e Mon Sep 17 00:00:00 2001 From: Daniel Wang <99078276+dantaik@users.noreply.github.com> Date: Sun, 10 Mar 2024 19:14:12 +0800 Subject: [PATCH 5/6] feat(protocol): allow minGuardians be any value between 0 and numGuardians (#16384) --- packages/protocol/contracts/L1/provers/Guardians.sol | 11 +++-------- packages/protocol/test/L1/Guardians.t.sol | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/protocol/contracts/L1/provers/Guardians.sol b/packages/protocol/contracts/L1/provers/Guardians.sol index d84a5fbac8..2796bce9a6 100644 --- a/packages/protocol/contracts/L1/provers/Guardians.sol +++ b/packages/protocol/contracts/L1/provers/Guardians.sol @@ -7,9 +7,6 @@ import "../../common/EssentialContract.sol"; /// @notice A contract that manages a set of guardians and their approvals. /// @custom:security-contact security@taiko.xyz abstract contract Guardians is EssentialContract { - /// @notice The minimum number of guardians - uint256 public constant MIN_NUM_GUARDIANS = 5; - /// @notice Contains the index of the guardian in `guardians` plus one (zero means not a /// guardian) /// @dev Slot 1 @@ -58,15 +55,13 @@ abstract contract Guardians is EssentialContract { onlyOwner nonReentrant { - // We need at least MIN_NUM_GUARDIANS and at most 255 guardians (so the approval bits fit in - // a uint256) - if (_newGuardians.length < MIN_NUM_GUARDIANS || _newGuardians.length > type(uint8).max) { + // We need at most 255 guardians (so the approval bits fit in a uint256) + if (_newGuardians.length == 0 || _newGuardians.length > type(uint8).max) { revert INVALID_GUARDIAN_SET(); } // Minimum number of guardians to approve is at least equal or greater than half the // guardians (rounded up) and less or equal than the total number of guardians - if (_minGuardians < (_newGuardians.length + 1) >> 1 || _minGuardians > _newGuardians.length) - { + if (_minGuardians == 0 || _minGuardians > _newGuardians.length) { revert INVALID_MIN_GUARDIANS(); } diff --git a/packages/protocol/test/L1/Guardians.t.sol b/packages/protocol/test/L1/Guardians.t.sol index d088b2551f..8f7021c5d9 100644 --- a/packages/protocol/test/L1/Guardians.t.sol +++ b/packages/protocol/test/L1/Guardians.t.sol @@ -15,7 +15,7 @@ contract DummyGuardians is Guardians { } } -contract TestSignalService is TaikoTest { +contract TestGuardianProver is TaikoTest { DummyGuardians target; function getSigners(uint256 numGuardians) internal returns (address[] memory signers) { From 627bf01fdc82d0fa86a1705920deae4e9b8dcdb3 Mon Sep 17 00:00:00 2001 From: Daniel Wang <99078276+dantaik@users.noreply.github.com> Date: Sun, 10 Mar 2024 20:02:53 +0800 Subject: [PATCH 6/6] chore(protocol): avoid copying any data when send Ether (#16361) --- .../contracts/L1/hooks/AssignmentHook.sol | 10 ++- .../contracts/L1/libs/LibDepositing.sol | 2 +- .../contracts/L1/libs/LibProposing.sol | 2 +- packages/protocol/contracts/L2/TaikoL2.sol | 2 +- packages/protocol/contracts/bridge/Bridge.sol | 17 ++--- .../protocol/contracts/libs/LibAddress.sol | 59 ++++++++++++----- .../nomad-xyz/ExcessivelySafeCall.sol | 64 ------------------- .../contracts/tokenvault/ERC1155Vault.sol | 4 +- .../contracts/tokenvault/ERC20Vault.sol | 4 +- .../contracts/tokenvault/ERC721Vault.sol | 4 +- 10 files changed, 63 insertions(+), 105 deletions(-) delete mode 100644 packages/protocol/contracts/thirdparty/nomad-xyz/ExcessivelySafeCall.sol diff --git a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol index e3239124dc..9591504ff7 100644 --- a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol +++ b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol @@ -31,6 +31,8 @@ contract AssignmentHook is EssentialContract, IHook { uint256 tip; // A tip to L1 block builder } + event EtherPaymentFailed(address to, uint256 maxGas); + /// @notice Max gas paying the prover. /// @dev This should be large enough to prevent the worst cases for the prover. /// To assure a trustless relationship between the proposer and the prover it's @@ -112,7 +114,9 @@ contract AssignmentHook is EssentialContract, IHook { // Ether or ERC20 tokens. if (assignment.feeToken == address(0)) { // Paying Ether - _blk.assignedProver.sendEther(proverFee, MAX_GAS_PAYING_PROVER); + // Note that this payment may fail if it cost more gas + bool success = _blk.assignedProver.sendEther(proverFee, MAX_GAS_PAYING_PROVER, ""); + if (!success) emit EtherPaymentFailed(_blk.assignedProver, MAX_GAS_PAYING_PROVER); } else { // Paying ERC20 tokens IERC20(assignment.feeToken).safeTransferFrom( @@ -122,12 +126,12 @@ contract AssignmentHook is EssentialContract, IHook { // block.coinbase can be address(0) in tests if (input.tip != 0 && block.coinbase != address(0)) { - address(block.coinbase).sendEther(input.tip); + address(block.coinbase).sendEtherAndVerify(input.tip); } // Send all remaining Ether back to TaikoL1 contract if (address(this).balance > 0) { - taikoL1Address.sendEther(address(this).balance); + taikoL1Address.sendEtherAndVerify(address(this).balance); } emit BlockAssigned(_blk.assignedProver, _meta, assignment); diff --git a/packages/protocol/contracts/L1/libs/LibDepositing.sol b/packages/protocol/contracts/L1/libs/LibDepositing.sol index 1d4c9fc969..9da9abc7de 100644 --- a/packages/protocol/contracts/L1/libs/LibDepositing.sol +++ b/packages/protocol/contracts/L1/libs/LibDepositing.sol @@ -38,7 +38,7 @@ library LibDepositing { revert L1_INVALID_ETH_DEPOSIT(); } - _resolver.resolve("bridge", false).sendEther(msg.value); + _resolver.resolve("bridge", false).sendEtherAndVerify(msg.value); // Append the deposit to the queue. address recipient_ = _recipient == address(0) ? msg.sender : _recipient; diff --git a/packages/protocol/contracts/L1/libs/LibProposing.sol b/packages/protocol/contracts/L1/libs/LibProposing.sol index d0c229c20d..e9813a683c 100644 --- a/packages/protocol/contracts/L1/libs/LibProposing.sol +++ b/packages/protocol/contracts/L1/libs/LibProposing.sol @@ -259,7 +259,7 @@ library LibProposing { } // Refund Ether if (address(this).balance != 0) { - msg.sender.sendEther(address(this).balance); + msg.sender.sendEtherAndVerify(address(this).balance); } // Check that after hooks, the Taiko Token balance of this contract diff --git a/packages/protocol/contracts/L2/TaikoL2.sol b/packages/protocol/contracts/L2/TaikoL2.sol index 8ad2c82829..98856876e5 100644 --- a/packages/protocol/contracts/L2/TaikoL2.sol +++ b/packages/protocol/contracts/L2/TaikoL2.sol @@ -171,7 +171,7 @@ contract TaikoL2 is CrossChainOwned { { if (_to == address(0)) revert L2_INVALID_PARAM(); if (_token == address(0)) { - _to.sendEther(address(this).balance); + _to.sendEtherAndVerify(address(this).balance); } else { IERC20(_token).safeTransfer(_to, IERC20(_token).balanceOf(address(this))); } diff --git a/packages/protocol/contracts/bridge/Bridge.sol b/packages/protocol/contracts/bridge/Bridge.sol index 02588459e6..13c6b11c31 100644 --- a/packages/protocol/contracts/bridge/Bridge.sol +++ b/packages/protocol/contracts/bridge/Bridge.sol @@ -5,7 +5,6 @@ import "@openzeppelin/contracts/utils/Address.sol"; import "../common/EssentialContract.sol"; import "../libs/LibAddress.sol"; import "../signal/ISignalService.sol"; -import "../thirdparty/nomad-xyz/ExcessivelySafeCall.sol"; import "./IBridge.sol"; /// @title Bridge @@ -203,7 +202,7 @@ contract Bridge is EssentialContract, IBridge { // Must reset the context after the message call _resetContext(); } else { - _message.srcOwner.sendEther(_message.value); + _message.srcOwner.sendEtherAndVerify(_message.value); } emit MessageRecalled(msgHash); } else if (!isMessageProven) { @@ -292,11 +291,11 @@ contract Bridge is EssentialContract, IBridge { // Refund the processing fee if (msg.sender == refundTo) { - refundTo.sendEther(_message.fee + refundAmount); + refundTo.sendEtherAndVerify(_message.fee + refundAmount); } else { // If sender is another address, reward it and refund the rest - msg.sender.sendEther(_message.fee); - refundTo.sendEther(refundAmount); + msg.sender.sendEtherAndVerify(_message.fee); + refundTo.sendEtherAndVerify(refundAmount); } emit MessageExecuted(msgHash); } else if (!isMessageProven) { @@ -494,13 +493,7 @@ contract Bridge is EssentialContract, IBridge { ) { success_ = false; } else { - (success_,) = ExcessivelySafeCall.excessivelySafeCall( - _message.to, - _gasLimit, - _message.value, - 64, // return max 64 bytes - _message.data - ); + success_ = _message.to.sendEther(_message.value, _gasLimit, _message.data); } // Must reset the context after the message call diff --git a/packages/protocol/contracts/libs/LibAddress.sol b/packages/protocol/contracts/libs/LibAddress.sol index 0219d78bfb..d82dbb71fc 100644 --- a/packages/protocol/contracts/libs/LibAddress.sol +++ b/packages/protocol/contracts/libs/LibAddress.sol @@ -5,7 +5,6 @@ import "@openzeppelin/contracts/utils/Address.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import "@openzeppelin/contracts/interfaces/IERC1271.sol"; -import "../thirdparty/nomad-xyz/ExcessivelySafeCall.sol"; /// @title LibAddress /// @dev Provides utilities for address-related operations. @@ -15,32 +14,58 @@ library LibAddress { error ETH_TRANSFER_FAILED(); - /// @dev Sends Ether to the specified address. + /// @dev Sends Ether to the specified address. This method will not revert even if sending ether + /// fails. + /// This function is inspired by + /// https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol /// @param _to The recipient address. /// @param _amount The amount of Ether to send in wei. /// @param _gasLimit The max amount gas to pay for this transaction. - function sendEther(address _to, uint256 _amount, uint256 _gasLimit) internal { + /// @return success_ true if the call is successful, false otherwise. + function sendEther( + address _to, + uint256 _amount, + uint256 _gasLimit, + bytes memory _calldata + ) + internal + returns (bool success_) + { // Check for zero-address transactions if (_to == address(0)) revert ETH_TRANSFER_FAILED(); + // dispatch message to recipient + // by assembly calling "handle" function + // we call via assembly to avoid memcopying a very large returndata + // returned by a malicious contract + assembly { + success_ := + call( + _gasLimit, // gas + _to, // recipient + _amount, // ether value + add(_calldata, 0x20), // inloc + mload(_calldata), // inlen + 0, // outloc + 0 // outlen + ) + } + } - // Attempt to send Ether to the recipient address - (bool success,) = ExcessivelySafeCall.excessivelySafeCall( - _to, - _gasLimit, - _amount, - 64, // return max 64 bytes - "" - ); - - // Ensure the transfer was successful - if (!success) revert ETH_TRANSFER_FAILED(); + /// @dev Sends Ether to the specified address. This method will revert if sending ether fails. + /// @param _to The recipient address. + /// @param _amount The amount of Ether to send in wei. + /// @param _gasLimit The max amount gas to pay for this transaction. + function sendEtherAndVerify(address _to, uint256 _amount, uint256 _gasLimit) internal { + if (!sendEther(_to, _amount, _gasLimit, "")) { + revert ETH_TRANSFER_FAILED(); + } } - /// @dev Sends Ether to the specified address. + /// @dev Sends Ether to the specified address. This method will revert if sending ether fails. /// @param _to The recipient address. /// @param _amount The amount of Ether to send in wei. - function sendEther(address _to, uint256 _amount) internal { - sendEther(_to, _amount, gasleft()); + function sendEtherAndVerify(address _to, uint256 _amount) internal { + sendEtherAndVerify(_to, _amount, gasleft()); } function supportsInterface( diff --git a/packages/protocol/contracts/thirdparty/nomad-xyz/ExcessivelySafeCall.sol b/packages/protocol/contracts/thirdparty/nomad-xyz/ExcessivelySafeCall.sol deleted file mode 100644 index e8c466ea12..0000000000 --- a/packages/protocol/contracts/thirdparty/nomad-xyz/ExcessivelySafeCall.sol +++ /dev/null @@ -1,64 +0,0 @@ -// SPDX-License-Identifier: MIT OR Apache-2.0 -// https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol -pragma solidity 0.8.24; - -library ExcessivelySafeCall { - uint256 constant LOW_28_MASK = - 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - - /// @notice Use when you _really_ really _really_ don't trust the called - /// contract. This prevents the called contract from causing reversion of - /// the caller in as many ways as we can. - /// @dev The main difference between this and a solidity low-level call is - /// that we limit the number of bytes that the callee can cause to be - /// copied to caller memory. This prevents stupid things like malicious - /// contracts returning 10,000,000 bytes causing a local OOG when copying - /// to memory. - /// @param _target The address to call - /// @param _gas The amount of gas to forward to the remote contract - /// @param _value The value in wei to send to the remote contract - /// @param _maxCopy The maximum number of bytes of returndata to copy - /// to memory. - /// @param _calldata The data to send to the remote contract - /// @return success and returndata, as `.call()`. Returndata is capped to - /// `_maxCopy` bytes. - function excessivelySafeCall( - address _target, - uint256 _gas, - uint256 _value, - uint16 _maxCopy, - bytes memory _calldata - ) - internal - returns (bool, bytes memory) - { - // set up for assembly call - uint256 _toCopy; - bool _success; - bytes memory _returnData = new bytes(_maxCopy); - // dispatch message to recipient - // by assembly calling "handle" function - // we call via assembly to avoid memcopying a very large returndata - // returned by a malicious contract - assembly { - _success := - call( - _gas, // gas - _target, // recipient - _value, // ether value - add(_calldata, 0x20), // inloc - mload(_calldata), // inlen - 0, // outloc - 0 // outlen - ) - // limit our copy to 256 bytes - _toCopy := returndatasize() - if gt(_toCopy, _maxCopy) { _toCopy := _maxCopy } - // Store the length of the copied bytes - mstore(_returnData, _toCopy) - // copy the bytes from returndata[0:_toCopy] - returndatacopy(add(_returnData, 0x20), 0, _toCopy) - } - return (_success, _returnData); - } -} diff --git a/packages/protocol/contracts/tokenvault/ERC1155Vault.sol b/packages/protocol/contracts/tokenvault/ERC1155Vault.sol index 5b9894837f..c91b992c9f 100644 --- a/packages/protocol/contracts/tokenvault/ERC1155Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC1155Vault.sol @@ -109,7 +109,7 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { // Transfer the ETH and the tokens to the `to` address address token = _transferTokens(ctoken, to, tokenIds, amounts); - to.sendEther(msg.value); + to.sendEtherAndVerify(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -143,7 +143,7 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, message.srcOwner, tokenIds, amounts); - message.srcOwner.sendEther(message.value); + message.srcOwner.sendEtherAndVerify(message.value); // Emit TokenReleased event emit TokenReleased({ diff --git a/packages/protocol/contracts/tokenvault/ERC20Vault.sol b/packages/protocol/contracts/tokenvault/ERC20Vault.sol index 0526a2eb67..fcf0804c05 100644 --- a/packages/protocol/contracts/tokenvault/ERC20Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC20Vault.sol @@ -268,7 +268,7 @@ contract ERC20Vault is BaseVault { // Transfer the ETH and the tokens to the `to` address address token = _transferTokens(ctoken, to, amount); - to.sendEther(msg.value); + to.sendEtherAndVerify(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -301,7 +301,7 @@ contract ERC20Vault is BaseVault { // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, _message.srcOwner, amount); - _message.srcOwner.sendEther(_message.value); + _message.srcOwner.sendEtherAndVerify(_message.value); emit TokenReleased({ msgHash: _msgHash, diff --git a/packages/protocol/contracts/tokenvault/ERC721Vault.sol b/packages/protocol/contracts/tokenvault/ERC721Vault.sol index 71dc04e119..a0329fe11d 100644 --- a/packages/protocol/contracts/tokenvault/ERC721Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC721Vault.sol @@ -92,7 +92,7 @@ contract ERC721Vault is BaseNFTVault, IERC721Receiver { // Transfer the ETH and the tokens to the `to` address address token = _transferTokens(ctoken, to, tokenIds); - to.sendEther(msg.value); + to.sendEtherAndVerify(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -126,7 +126,7 @@ contract ERC721Vault is BaseNFTVault, IERC721Receiver { // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, _message.srcOwner, tokenIds); - _message.srcOwner.sendEther(_message.value); + _message.srcOwner.sendEtherAndVerify(_message.value); emit TokenReleased({ msgHash: _msgHash,