-
Notifications
You must be signed in to change notification settings - Fork 62
ENG-472 Bridge controller pointer and MintBurnGuard guardrails #918
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
Open
piotr-roslaniec
wants to merge
79
commits into
main
Choose a base branch
from
new/bank-decreaser
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 32 commits
Commits
Show all changes
79 commits
Select commit
Hold shift + click to select a range
da0c4dc
feat: extend bank balance authorization
piotr-roslaniec c7bf48a
docs: add Bridge controller-authorization upgrade explainer; ignore l…
piotr-roslaniec 0f5f209
deploy(upgrade-bridge): prefer cached deployment, use PROXY_ADMIN_PK …
piotr-roslaniec 3cb3a0a
deploy(transfer-governance): use governance signer (BRIDGE_GOVERNANCE…
piotr-roslaniec 20a8e8f
scripts(sepolia): call BridgeGovernance ownership transfer; fix rollb…
piotr-roslaniec 2c777b0
config(hardhat): set optimizer runs to 200; update comments and keep …
piotr-roslaniec 1976be0
deploy(governance): conditional Tenderly/Etherscan verify and minor p…
piotr-roslaniec 9d56c1b
chore(docs): format bridge upgrade explainer with Prettier
piotr-roslaniec 8969dac
chore(solidity): relax lint for deploy/scripts via overrides; fix typ…
piotr-roslaniec 766a6d0
Sepolia: redeploy BridgeGovernance and verify allowlist
piotr-roslaniec c92f50a
Remove unused solidity .env example
piotr-roslaniec d74bb6b
Remove Sepolia-specific upgrade/rollback scripts
piotr-roslaniec 38159ee
Revert "Sepolia: redeploy BridgeGovernance and verify allowlist"
piotr-roslaniec 5dec1e5
Remove toggle-increaser helper script
piotr-roslaniec 1cf6301
Remove verify-sepolia helper script
piotr-roslaniec 66ec3bd
chore(governance): helper + safer controller sync
piotr-roslaniec 1aa5af7
docs: consolidate controller upgrade and governance redeploy
piotr-roslaniec b3d4f0a
docs: remove redundant bridge governance redeploy report
piotr-roslaniec c729bc9
solidity: switch git+ssh deps to git+https
piotr-roslaniec 874500c
ci(contracts): use node 18 for solidity jobs
piotr-roslaniec 64aa9e7
ci(contracts): align format/slither with node 18
piotr-roslaniec 3fc56bb
ci(contracts): move solidity jobs to node 20
piotr-roslaniec 1c80f74
chore(governance): controller ops tooling and docs
piotr-roslaniec dc44a82
chore(solidity): apply Prettier formatting
piotr-roslaniec 0312bab
chore(solidity): align hardhat and deploy tooling with main
piotr-roslaniec 3369741
chore(solidity): revert unrelated contract formatting
piotr-roslaniec 09e95dd
Fix bridge governance CI and formatting
piotr-roslaniec c460c8d
Use Node 18 for contracts CI
piotr-roslaniec eb6fa37
chore(governance): harden controller sync and tooling
piotr-roslaniec f5773f7
ci(docs): ignore engines for solidity docs workflow
piotr-roslaniec 33993a4
ci(docs): set YARN_IGNORE_ENGINES at workflow level
piotr-roslaniec e5991a3
ci(contracts): run format job on node 18 and ignore generated docs
piotr-roslaniec 52a01d7
feat(bridge): controller caps and MintingGuard scaffold
piotr-roslaniec 1b9d275
refactor(bridge): rely on MintingGuard for caps
piotr-roslaniec 4831a07
Wire MintingGuard executor and docs for controller allowlist
piotr-roslaniec 1d01c9f
test: relax MintingGuard revert assertions
piotr-roslaniec 3fc3497
slither: silence expected reentrancy warnings for controllers
piotr-roslaniec f4b94c8
slither: widen controller reentrancy suppression
piotr-roslaniec 8a4c28b
refactor: emit controller events before bank calls
piotr-roslaniec a3ac589
Wire MintBurnGuard interface, tests, and docs
piotr-roslaniec 682bb52
chore: rename MintingGuard to MintBurnGuard
piotr-roslaniec 7ccab01
chore: switch bridge to single controller
piotr-roslaniec 593b0a9
fix: share vault interface and satisfy slither
piotr-roslaniec b72596f
refactor: keep vault interface local
piotr-roslaniec 505b08a
Use ITBTCVault in MintBurnGuard and fix lints
piotr-roslaniec af0d715
Improve execution wiring and governance transfer workflow
piotr-roslaniec d5c995e
Allow internal wiring helpers
piotr-roslaniec 963b24a
refactor: reuse guard increase logic
piotr-roslaniec d911874
Add configurable mint rate limiting to MintBurnGuard
piotr-roslaniec 44468af
Suppress time-based lint for MintBurnGuard rate limit
piotr-roslaniec a15a87e
Document MintBurnGuard rate limiting
codex fdfa279
fix(mintburn): avoid window start equality
piotr-roslaniec 7a69404
Merge remote-tracking branch 'origin/main' into new/bank-decreaser
piotr-roslaniec 41d7a8e
fix(mintburnguard): address audit findings ENG-473
piotr-roslaniec 3526421
Merge branch 'main' into feat/eng-472-mintburnguard-workstream
miquelcabot a1fa407
Implement one-off governance function setControllerBalanceIncreaser
miquelcabot c1a1195
refactor: rename controller-related interfaces, functions and events
miquelcabot fe7d9af
refactor: update MintBurnGuard to use 'operator' terminology instead …
miquelcabot 76a2869
feat: implement MintBurnGuard deployment and configuration scripts
miquelcabot 1b794d2
refactor: add unmintAndBurnFrom and burnFrom functions to MintBurnGuard
miquelcabot 764ea30
refactor: enhance unmintAndBurnFrom and burn functions with SafeERC20…
miquelcabot 1281655
refactor: add error handling for bank transfer failure in MintBurnGuard
miquelcabot b2aaf13
test: add operator change functionality tests in MintBurnGuard
miquelcabot 89c6cd1
refactor: rename controller to operator in MintBurnGuard tests
miquelcabot 08407c1
feat: implement MockAccountControl for testing MintBurnGuard integration
miquelcabot f5bc30d
feat: add bank and tbtcToken addresses for improved vault integration…
miquelcabot cee32f0
refactor: change TBTC token approval to use safeIncreaseAllowance in …
miquelcabot 158a8d1
fix: update balance handling in MintBurnGuard to use satoshi (1e8) am…
miquelcabot 3675843
Merge branch 'main' into new/bank-decreaser
miquelcabot 5bc750e
Merge branch 'new/bank-decreaser' into feat/eng-472-mintburnguard-wor…
miquelcabot 0d0613a
feat: add bridge address to MintBurnGuard for improved vault integration
miquelcabot e9913cb
feat: refactor MintBurnGuard to use upgradeable proxy pattern and add…
miquelcabot 2533910
feat: update Slither version to 0.10.1
miquelcabot a5dcc8a
feat: update Slither version to 0.11.0
miquelcabot d0be3d8
feat: add 'incorrect-equality' to Slither detectors to exclude
miquelcabot 6b56b55
feat: update Slither command to fail on high severity issues
miquelcabot b7b33f4
feat: add integration tests for MintBurnGuard contract interactions
miquelcabot ff9e4f2
ENG-472: MintBurnGuard workstream. Governance, interfaces, integratio…
piotr-roslaniec 6ba652b
fix(bridge): re-upgrade Bridge proxy to MintBurnGuard-aware impleme…
antomor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| # Bridge Upgrade: Controller Authorization Allowlist (Explainer + Sepolia Report) | ||
|
|
||
| ## Summary | ||
|
|
||
| - Objective: enable a governance‑managed allowlist of controller contracts that can mint via the Bridge by increasing Bank balances. | ||
| - Approach: upgrade the Bridge proxy implementation to add the allowlist and controller entrypoints; redeploy BridgeGovernance with a forwarder function; transfer governance; optionally sync the allowlist. | ||
| - Safety: evented changes, explicit zero‑address checks, governance‑only setters, snapshot + rollback tooling. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Integrations need a narrow, auditable way to mint balances through the Bridge without broad privileges. Introducing an “authorized balance increaser” allowlist lets governance approve specific controller contracts to call controlled minting functions, minimizing surface area while preserving the existing flow and roles. | ||
|
|
||
| This model provides: | ||
|
|
||
| - Least‑privilege controller minting gate on the Bridge. | ||
| - On‑chain audit trail via events. | ||
| - Operationally simple management via BridgeGovernance. | ||
|
|
||
| ## Trust Model & Operational Guardrails | ||
|
|
||
| - Controllers are high‑privilege actors. Any address authorized in the | ||
| `authorizedBalanceIncreasers` mapping can increase arbitrary Bank balances via | ||
| the Bridge and should be treated as having governance‑level minting power. | ||
| - Only fully reviewed and audited contracts should ever be added as | ||
| controllers. In particular, controller contracts must not expose generic | ||
| "increase balance" surfaces to untrusted callers and should implement their | ||
| own internal policy checks (limits, roles, pause switches) as appropriate. | ||
| - Governance is responsible for keeping the allowlist tight and | ||
| human‑auditable: | ||
| - Additions and removals must be performed through BridgeGovernance, which | ||
| emits `AuthorizedBalanceIncreaserUpdated` events for each change. | ||
| - Off‑chain monitoring should alert on any unexpected controller additions, | ||
| removals, or large `BalanceIncreased` events. | ||
| - The controller sync tooling is explicitly conservative by default: | ||
| - When no desired controller list is provided, existing authorizations are | ||
| left unchanged unless mass‑revoke is explicitly enabled. | ||
| - Mass revocation requires an explicit opt‑in via either a function | ||
| parameter or the `BRIDGE_ALLOW_MASS_CONTROLLER_REVOKE=true` env flag. | ||
|
|
||
| Operationally, controllers should be treated as part of the governance surface | ||
| area and managed via the same change‑management process (multi‑sig, change | ||
| review, deployment runbooks, and monitoring). | ||
|
|
||
| ## What Changed (Contracts) | ||
|
|
||
| - Bridge (proxy): | ||
| - New state: `authorizedBalanceIncreasers` mapping (storage layout extended). | ||
| - New events: `AuthorizedBalanceIncreaserUpdated(address,bool)`. | ||
| - New methods (gated at runtime by allowlist): | ||
| - `controllerIncreaseBalance(address,uint256)` | ||
| - `controllerIncreaseBalances(address[],uint256[])` | ||
| - `authorizedBalanceIncreasers(address) -> bool` | ||
| - New governance setter: `setAuthorizedBalanceIncreaser(address,bool)` (onlyGovernance). | ||
| - BridgeGovernance (regular contract): | ||
| - New owner‑only forwarder: `setAuthorizedBalanceIncreaser(address,bool)` that calls into Bridge. | ||
| - New interface for integrators: `IBridgeMintingAuthorization` (minimal surface consumed by account‑control). | ||
|
|
||
| ## Why Governance Redeploy Is Needed | ||
|
|
||
| - Bridge’s governance role is the BridgeGovernance contract address. To manage the new allowlist on Bridge, BridgeGovernance itself must expose the matching forwarder function. | ||
| - The legacy governance contract does not have it, so we deploy a fresh BridgeGovernance and transfer Bridge governance to the new instance (governance delay applies). | ||
|
|
||
| ## Upgrade Plan (High‑Level) | ||
|
|
||
| 1. Pre‑upgrade snapshot of Bridge state (implementation/admin/governance, parameters, allowlists). | ||
| 2. Upgrade Bridge proxy implementation via ProxyAdmin to the version with controller allowlist. | ||
| 3. Redeploy BridgeGovernance (fresh instance) and transfer governance: | ||
| - Begin transfer, wait governance delay, finalize. | ||
| 4. Optionally sync authorized controllers from env/config; emit events for adds/removals. | ||
| 5. Post‑upgrade snapshot; compare and archive. | ||
|
|
||
| Supporting scripts (names as in repo): | ||
|
|
||
| - `solidity/deploy/80_upgrade_bridge_v2.ts` — upgrade Bridge, resolve libraries/addresses, conditional Tenderly verify. | ||
| - `solidity/deploy/09_deploy_bridge_governance.ts` — deploy BridgeGovernance (+Parameters), conditional Tenderly verify. | ||
| - `solidity/deploy/21_transfer_bridge_governance.ts` + `solidity/deploy/utils/governance-transfer.ts` — initiate/finalize governance transfer while respecting the governance delay (with optional begin‑only/finalize‑only modes for long delays). | ||
| - `solidity/scripts/configure-bridge-controllers.ts` + `solidity/deploy/utils/bridge-controller-authorization.ts` — sync the controller allowlist from env when explicitly invoked as a Hardhat script. | ||
|
|
||
| ## Risks & Mitigations | ||
|
|
||
| - Storage layout changes: uses mapped slot and reduces the storage gap accordingly; upgrade path accounted for in implementation. | ||
| - Misconfiguration risk: snapshot + rollback scripts provided; allowlist sync is explicit and evented. | ||
| - Tenderly availability: verification is conditional on local Tenderly config to avoid deployment failures. | ||
|
|
||
| ## Environment Notes | ||
|
|
||
| - Env keys used during orchestration include: `BRIDGE_ADDRESS`, `PROXY_ADMIN_PK`, `BRIDGE_GOVERNANCE_PK`, `BRIDGE_AUTHORIZED_INCREASERS`, and library/core contract address fallbacks. | ||
| - Mass revocation safeguard: to revoke all existing controller authorizations when no `BRIDGE_AUTHORIZED_INCREASERS` are provided, set `BRIDGE_ALLOW_MASS_CONTROLLER_REVOKE=true` (otherwise existing authorizations are left unchanged). | ||
| - Sepolia RPC: prefer `SEPOLIA_CHAIN_API_URL`/`SEPOLIA_PRIVATE_KEYS` where applicable. | ||
| - Governance transfer helper: `BRIDGE_GOVERNANCE_TRANSFER_MODE` can be set to `begin`, `finalize`, or left unset (default `full`) to control how `21_transfer_bridge_governance.ts` orchestrates begin/finalize steps for long governance delays. | ||
|
|
||
| --- | ||
|
|
||
| ## Appendix A: Sepolia Execution Report – Controller Allowlist Upgrade | ||
|
|
||
| The following section preserves the original Sepolia run report for traceability. | ||
|
|
||
| ### Overview | ||
|
|
||
| - Proxy address: `0x9b1a7fE5a16A15F2f9475C5B231750598b113403` | ||
| - New implementation: `0x1c19BBF9afAfe5e8EA4F78c1178752cE62683694` | ||
| - Proxy admin: `0x39f60B25C4598Caf7e922d6fC063E9002db45845` | ||
| - New BridgeGovernance: `0x78c99F5B8981A7FDa02E9700778c8493b2eb7D6b` | ||
| - Upgrade signer: `0x68ad60CC5e8f3B7cC53beaB321cf0e6036962dBc` | ||
| - Governance owner: `0xF4767Aecf1dFB3a63791538E065c0C9F7f8920C3` | ||
|
|
||
| ### Actions Performed | ||
|
|
||
| 1. Bridge proxy upgrade | ||
| Executed `ProxyAdmin.upgrade` (tx `0x05e00adfc9f091443eb44ea619cac497ff9aa32a27e49539716a93ae8ed5a7fd`), swapping the Bridge proxy’s implementation to `0x1c19…`. The proxy address and admin remained unchanged (`deployments/sepolia/Bridge.json`). | ||
|
|
||
| 2. BridgeGovernance redeployment | ||
| Deployed a fresh governance contract at `0x78c99F5…` with governance delay `60` seconds and finalized ownership to the treasury signer (`deployments/sepolia/BridgeGovernance.json`). | ||
|
|
||
| 3. Environment updates | ||
| Updated `.env` and `.env.sepolia` to point at the new governance address and to use the treasury signer private key for governance actions. | ||
|
|
||
| 4. Snapshots & tooling | ||
| Captured pre/post-upgrade snapshots in an internal upgrade log and verified proxy admin / implementation slots to confirm the upgrade. | ||
|
|
||
| ### Post‑Upgrade State Verification | ||
|
|
||
| - `Bridge` proxy implementation slot resolves to `0x1c19…`; proxy admin slot remains `0x39f60B25…`. | ||
| - `Bridge.governance()` returns the new governance address `0x78c99F5…`. | ||
| - Bridge parameter structs, trusted vault list (vault `0xB5679dE…`), and SPV maintainers (`0x3Bc9a80…`, `0x68ad60…`) match pre-upgrade values. | ||
| - BridgeGovernance reports the treasury signer as owner and retains the 60 second governance delay. | ||
|
|
||
| ### Summary | ||
|
|
||
| The Sepolia bridge stack now runs the refreshed Bridge implementation behind the existing proxy while delegating governance to the newly deployed BridgeGovernance contract. All intended configuration, allowlists, and operational parameters were carried forward without deviation. No outstanding issues were observed. | ||
|
|
||
| --- | ||
|
|
||
| ## Appendix B: Sepolia Execution Report – BridgeGovernance Redeploy (2025‑11‑16) | ||
|
|
||
| This appendix summarizes a later Sepolia operation where only `BridgeGovernance` was redeployed from current sources, without changing the Bridge proxy or its parameters, in order to restore a verifiable governance contract that can manage the controller allowlist. | ||
|
|
||
| ### Overview | ||
|
|
||
| - Date (UTC): 2025-11-16 | ||
| - Network: Sepolia (chainId 11155111) | ||
|
|
||
| ### Objective | ||
|
|
||
| Redeploy `BridgeGovernance` from current sources and transfer `Bridge.governance()` to the new contract so that the controller allowlist (`setAuthorizedBalanceIncreaser`) can be used and verified, without changing the Bridge proxy or its parameters. | ||
|
|
||
| ### Before | ||
|
|
||
| - Bridge proxy: `0x9b1a7fE5a16A15F2f9475C5B231750598b113403` | ||
| - Bridge implementation (EIP-1967): `0x1c19BBF9afAfe5e8EA4F78c1178752cE62683694` | ||
| - Bridge governance: `0xAe0A3Fdfc51718E0952b3BcC03f672eB13917558` (unverified and not reproducible from git) | ||
|
|
||
| Parameters (deposit, redemption, moving funds, wallet), treasury and external references (Bank, Relay, WalletRegistry, ReimbursementPool) were all in the expected state and preserved. | ||
|
|
||
| ### Actions | ||
|
|
||
| - Deployed new `BridgeGovernance` from current sources. | ||
| - Address: `0x459FcE83bF5CF413D793D5bBD79E81010e8599c2` | ||
| - Constructor args: `(bridge = 0x9b1a7fE5a16A15F2f9475C5B231750598b113403, governanceDelay = 60)` | ||
| - Transferred governance on Bridge (old → new) using the old governance owner (`0x68ad60CC5e8f3B7cC53beaB321cf0e6036962dBc`). | ||
| - After transfer: `Bridge.governance()` returns `0x459F…` | ||
| - Verified contracts on Sepolia via `npx hardhat verify`: | ||
| - `BridgeGovernanceParameters` at `0x38CF632a41411e45d1c55A8F8E2586c8a69b2BB1` | ||
| - Bridge implementation at `0x32498B20c542eAd1207006bdAe8D9D0085c6cd39` | ||
| - New `BridgeGovernance` at `0x459F…` (`https://sepolia.etherscan.io/address/0x459FcE83bF5CF413D793D5bBD79E81010e8599c2#code`) | ||
| - Smoke-tested the controller allowlist: | ||
| - Called `setAuthorizedBalanceIncreaser(0x0000000000000000000000000000000000000001, true)` via the new governance owner; confirmed `authorizedBalanceIncreasers(testAddr) == true`, then reverted it back to `false`. | ||
|
|
||
| ### After | ||
|
|
||
| - Bridge proxy: `0x9b1a7fE5a16A15F2f9475C5B231750598b113403` (unchanged) | ||
| - Bridge implementation: `0x32498B20c542eAd1207006bdAe8D9D0085c6cd39` (contains allowlist entrypoints) | ||
| - Bridge governance: `0x459FcE83bF5CF413D793D5bBD79E81010e8599c2` | ||
| - Governance owner: `0x68ad60CC5e8f3B7cC53beaB321cf0e6036962dBc` | ||
| - No controllers are currently authorized; the allowlist feature is live and verified, but left empty by design. | ||
|
|
||
| Snapshots before and after the change are recorded in the internal upgrade log used for Sepolia operations (not committed to this repository). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,3 +8,5 @@ hardhat-dependency-compiler/ | |
| typechain/ | ||
| docgen-templates/ | ||
| export.json | ||
| .env | ||
| .env.* | ||
37 changes: 37 additions & 0 deletions
37
solidity/contracts/account-control/interfaces/IBridgeMintingAuthorization.sol
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // SPDX-License-Identifier: GPL-3.0-only | ||
|
|
||
| // ██████████████ ▐████▌ ██████████████ | ||
| // ██████████████ ▐████▌ ██████████████ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
| // ██████████████ ▐████▌ ██████████████ | ||
| // ██████████████ ▐████▌ ██████████████ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
| // ▐████▌ ▐████▌ | ||
|
|
||
| pragma solidity 0.8.17; | ||
|
|
||
| /// @notice Minimal Bridge surface consumed by AccountControl for minting. | ||
|
||
| interface IBridgeMintingAuthorization { | ||
| function controllerIncreaseBalance(address recipient, uint256 amount) | ||
| external; | ||
|
|
||
| function controllerIncreaseBalances( | ||
| address[] calldata recipients, | ||
| uint256[] calldata amounts | ||
| ) external; | ||
|
|
||
| function authorizedBalanceIncreasers(address increaser) | ||
| external | ||
| view | ||
| returns (bool); | ||
|
|
||
| function getAuthorizedBalanceIncreasers(address[] calldata increasers) | ||
| external | ||
| view | ||
| returns (bool[] memory); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.