Skip to content

Narrow bond and broker roles; hand deployer admin to the owner - #303

Merged
Greaker merged 1 commit into
feature/remove-bond-order-bookfrom
feature/bond-roles-least-privilege
Sep 28, 2026
Merged

Greaker merged 1 commit into
feature/remove-bond-order-bookfrom
feature/bond-roles-least-privilege

Conversation

@Greaker

@Greaker Greaker commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • BondToken.addController now only makes an address an ERC-1410 controller; lifecycle rights come from BOND_CONTROLLER_ROLE, granted separately. New removeController (BOND_ADMIN_ROLE).
  • Bond setup gives BondManager BOND_CONTROLLER_ROLE only and makes BondDvP a controller plus role holder for settlement, redemption and buyback. After wiring, the deployer grants DEFAULT_ADMIN_ROLE on BondManager, BondAuction, BondToken and BondDvP (and BOND_ADMIN_ROLE on BondToken) to the Norges Bank owner key and renounces its own.
  • BondManager constructor rejects zero wNOK, controller, auction, bond token and DvP addresses (new BondAuctionAddressZero).
  • OrderBook.getOrder view; Broker lets a client revoke only orders for that client's own securities wallet (OrderNotOwnedByClient). The OrderBook script grants SUBMIT_ORDER_ROLE to the Broker contracts only.
  • NB Bond API BondToken and BondManager ABIs refreshed; contract reference, security notes and NatSpec describe the role shape. Changes deployed bytecode and scripts.

Stacked on #302 (base feature/remove-bond-order-book); merge after it.

Test plan

  • cd contracts && forge fmt --check && forge build --sizes && bash ./check-verify-latest-mapping.sh && forge test -vvv (358 passed)
  • cd services/nb-bond-api && npm run lint && npm run format:check && npm test && npm run build (jest 254)
  • python3 scripts/verification/check-public-repo-hygiene.py && python3 scripts/verification/check-markdown-links.py
  • Contracts CI (Slither) green
  • After a fresh local deploy: deployer holds no admin role on the four bond contracts; owner key does

BondToken.addController now only makes an address an ERC-1410
controller (an operator over every holder's units); lifecycle rights
come from BOND_CONTROLLER_ROLE, granted separately. A new
removeController (BOND_ADMIN_ROLE) takes controller status away again.
The bond setup script gives BondManager BOND_CONTROLLER_ROLE only, since
it moves units solely through BondDvP, and makes BondDvP both a
controller and a BOND_CONTROLLER_ROLE holder for settlement, redemption
and buyback. Once wiring is done, the deployer key grants
DEFAULT_ADMIN_ROLE on BondManager, BondAuction, BondToken and BondDvP
(and BOND_ADMIN_ROLE on BondToken) to the Norges Bank owner key and
renounces its own, so no deployment-only key keeps admin on the bond
contracts while roles can still be rotated.

The BondManager constructor now rejects a zero wNOK, controller,
auction, bond token or DvP address, not only a zero reserve, adding
BondAuctionAddressZero.

OrderBook gains a getOrder view, and Broker lets a client revoke only
orders placed for that client's own securities wallet
(OrderNotOwnedByClient otherwise). The OrderBook script stops granting
SUBMIT_ORDER_ROLE to the broker EOAs; only the Broker contracts hold it.

The NB Bond API BondToken and BondManager ABIs are refreshed for the new
function and errors. Contract reference, security notes and NatSpec
pages describe the role shape.

Tests: forge 358 passed (new: controller add without lifecycle role,
controller removal and its effect on operator transfers, BondManager
constructor zero checks in a separate small suite, and cross-client
revoke rejection for buy and sell orders); nb-bond-api jest 254 passed.
@Greaker
Greaker merged commit 675d354 into feature/remove-bond-order-book Sep 28, 2026
5 checks passed
@Greaker
Greaker deleted the feature/bond-roles-least-privilege branch September 28, 2026 12:43
Greaker added a commit that referenced this pull request Sep 28, 2026
BondToken.addController now only makes an address an ERC-1410
controller (an operator over every holder's units); lifecycle rights
come from BOND_CONTROLLER_ROLE, granted separately. A new
removeController (BOND_ADMIN_ROLE) takes controller status away again.
The bond setup script gives BondManager BOND_CONTROLLER_ROLE only, since
it moves units solely through BondDvP, and makes BondDvP both a
controller and a BOND_CONTROLLER_ROLE holder for settlement, redemption
and buyback. Once wiring is done, the deployer key grants
DEFAULT_ADMIN_ROLE on BondManager, BondAuction, BondToken and BondDvP
(and BOND_ADMIN_ROLE on BondToken) to the Norges Bank owner key and
renounces its own, so no deployment-only key keeps admin on the bond
contracts while roles can still be rotated.

The BondManager constructor now rejects a zero wNOK, controller,
auction, bond token or DvP address, not only a zero reserve, adding
BondAuctionAddressZero.

OrderBook gains a getOrder view, and Broker lets a client revoke only
orders placed for that client's own securities wallet
(OrderNotOwnedByClient otherwise). The OrderBook script stops granting
SUBMIT_ORDER_ROLE to the broker EOAs; only the Broker contracts hold it.

The NB Bond API BondToken and BondManager ABIs are refreshed for the new
function and errors. Contract reference, security notes and NatSpec
pages describe the role shape.

Tests: forge 358 passed (new: controller add without lifecycle role,
controller removal and its effect on operator transfers, BondManager
constructor zero checks in a separate small suite, and cross-client
revoke rejection for buy and sell orders); nb-bond-api jest 254 passed.
Greaker added a commit that referenced this pull request Sep 28, 2026
* Remove the unused BondOrderBook and its factory

BondOrderBook and BondOrderBookFactory were never deployed by the sandbox
scripts and have no caller in the API or UI. The broker-level venue in
the closed-loop settlement plan is meant to replace them, so keeping and
maintaining a second, holder-level bond secondary market in source is
unused code rather than a supported feature. Both contracts, their test
suites and their NatSpec pages are removed.

TbdAddressZero and PartitionZero were only raised by the removed
contracts and are dropped from Errors; BondTokenAddressZero moves under
the BondManager heading. With this change the bond stack no longer
imports anything from the private-bank contracts.

The contract reference, the contract topology note, the NB Bond API
development guide and the active closed-loop settlement plan now state
that no on-chain bond secondary venue exists.

Tests: forge 347 passed (the 48 removed tests covered only the deleted
contracts).

* Narrow bond and broker roles; hand deployer admin to the owner (#303)

BondToken.addController now only makes an address an ERC-1410
controller (an operator over every holder's units); lifecycle rights
come from BOND_CONTROLLER_ROLE, granted separately. A new
removeController (BOND_ADMIN_ROLE) takes controller status away again.
The bond setup script gives BondManager BOND_CONTROLLER_ROLE only, since
it moves units solely through BondDvP, and makes BondDvP both a
controller and a BOND_CONTROLLER_ROLE holder for settlement, redemption
and buyback. Once wiring is done, the deployer key grants
DEFAULT_ADMIN_ROLE on BondManager, BondAuction, BondToken and BondDvP
(and BOND_ADMIN_ROLE on BondToken) to the Norges Bank owner key and
renounces its own, so no deployment-only key keeps admin on the bond
contracts while roles can still be rotated.

The BondManager constructor now rejects a zero wNOK, controller,
auction, bond token or DvP address, not only a zero reserve, adding
BondAuctionAddressZero.

OrderBook gains a getOrder view, and Broker lets a client revoke only
orders placed for that client's own securities wallet
(OrderNotOwnedByClient otherwise). The OrderBook script stops granting
SUBMIT_ORDER_ROLE to the broker EOAs; only the Broker contracts hold it.

The NB Bond API BondToken and BondManager ABIs are refreshed for the new
function and errors. Contract reference, security notes and NatSpec
pages describe the role shape.

Tests: forge 358 passed (new: controller add without lifecycle role,
controller removal and its effect on operator transfers, BondManager
constructor zero checks in a separate small suite, and cross-client
revoke rejection for buy and sell orders); nb-bond-api jest 254 passed.
@Greaker Greaker mentioned this pull request Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant