Identify potential improvement ideas for smart contracts

[nikipapadatou]

Based on the documented list of Document current and planned smart contracts, provide potential ideas for improvement or redesign — along with a short rationale for each.

For each improvement idea we need:

• Description of the improvement.
• Outline the expected benefits (e.g., performance, maintainability, decentralization, security, etc.).
• Rough time estimation. (S/M/L)
• High-level risks/trade-offs (e.g., gas costs, complexity, upgrade risk). - if applicable
• Priority score.

This work is exploratory; no implementation is expected.

The deliverable should be a project canvas summarizing the above information, linked to this issue.

[0xCardiE]

There are no potential improvements discussed for current smart contracts. We did a lot some year and half ago. I was optimizing code structure and gas usage and the whole deployment process. Also checked some security in process there.
So this was considered done at that point. What we could do is work more on security checks and talk about possible outside audit from audit companies.

[janos]

I would like to add a short feedback from the contracts usage/integration, particularly in the bee node implementation.

Given that there are a few contracts that are used in the node client, we keep their addresses in the configuration. For mainnet and testnet in the abi repositories. Synchronising updates is now explicit and require releasing new bee versions to update contracts. Sometimes that may not be needed. By using the proxy contract that will keep track of all swarm contracts addresses, smart contract could gain more flexibility related to their coupling with the node implementation and also the referencing of smart contracts by using only one proxy smart contract address would be more reliable from the node point of view.

I have a feeling that proxy smart contract could improve contracts usage and updating.

[mfw78]

I would like to add a short feedback from the contracts usage/integration, particularly in the bee node implementation.

Given that there are a few contracts that are used in the node client, we keep their addresses in the configuration. For mainnet and testnet in the abi repositories. Synchronising updates is now explicit and require releasing new bee versions to update contracts. Sometimes that may not be needed. By using the proxy contract that will keep track of all swarm contracts addresses, smart contract could gain more flexibility related to their coupling with the node implementation and also the referencing of smart contracts by using only one proxy smart contract address would be more reliable from the node point of view.

I have a feeling that proxy smart contract could improve contracts usage and updating.

This works to a degree, but when there's substantial change to storage kept in the contracts, this becomes very much non-trivial with many foot-guns. It would essentially yield greatest benefits if there are logic-only changes, with no underlying storage changes. If new functionality / storage were added this would likely then necessitate some form of migration itself to clean-up the storage on-chain.

I would actually suggest to follow a registry pattern instead. This would work something like:

1. Contracts are deployed.
2. A 'registry' contract is deployed.
3. Contracts deployed in (1) are stored against some mapping in (2).

For example, Registry.set("POSTAGE_STAMP") = 0xbadcodebadcode.... Now subsequently within bee code when there is startup, the addresses can be retrieved by consulting the Registry.

This leaves one with ensuring that only the ABI remains relatively stable, and would allow for non-breaking migration paths to be carried out.

[mfw78]

An example of where this pattern has been used extensively: https://github.com/sky-ecosystem/dss-chain-log

[janos]

I would actually suggest to follow a registry pattern instead. This would work something like:

1. Contracts are deployed.
2. A 'registry' contract is deployed.
3. Contracts deployed in (1) are stored against some mapping in (2).

For example, Registry.set("POSTAGE_STAMP") = 0xbadcodebadcode.... Now subsequently within bee code when there is startup, the addresses can be retrieved by consulting the Registry.

This leaves one with ensuring that only the ABI remains relatively stable, and would allow for non-breaking migration paths to be carried out.

Yes, I agree. The registry pattern gives all benefits of easier referencing, but keeping the abi strict. Thank you @mfw78.

[0xCardiE]

I was proposing using proxy patterns some 2 years ago, there were heavy discussions about this, made some proofs of concepts but it was decided that we want to stick with current pattern and not change it. So this probably wont be changing.

Reasons against proxy would mostly apply also for registry patterns which were basically that we dont want to have control on what code is used in nodes, meaning if we can change logic in proxies or change values in registry without user consent for their nodes this is something that we dont want to do or have option to.

[janos]

That argument is strong. At least, now, the user can validate all the changes before the node upgrade. The assumption if users are actually doing that is not relevant if silent mutability of contracts if forbidden as a principle. What if the registry contract is only providing information about other contracts that are set on contract deployment and could not be changed. So that mutability is not possible, but there is only one address to reference a set of contracts to be more reliable as a coherent suit of contracts (not to mistakenly mix contracts that are not "compatible")?

[mfw78]

Just pass along the version as well that bee is running with Register.get('COMPONENT', '2.6.0'). In which case, upgrading the node is an implicit agreement to the new contracts (if applicable). This still provides substantial benefits by not having to bump in multiple places and provides reasonable logical separation and division along 'areas of responsibility' for node developers and smart contracts.

[0xCardiE]

I am think would registry still have similar problem as with proxies. If we can change the registry as admins then we could change values without user knowing it.

So I am thinking that we could try to enforce maping with setOnce method like, then registry would make sense to use as some truth oracle.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

contract BeeContractsRegistry {
    error VersionAlreadyRegistered();
    error ZeroAddress();
    error NotAuthorized();

    address public immutable admin;

    constructor(address _admin) {
        admin = _admin;
    }

    struct Bundle {
        address postageStamp;
        address redistribution;
        address staking;
        address priceOracle;
    }

    // versionKey (keccak256(versionString)) => Bundle
    mapping(bytes32 => Bundle) private bundles;

    event VersionRegistered(
        bytes32 indexed versionKey,
        string versionString,
        address postageStamp,
        address redistribution,
        address staking,
        address priceOracle
    );

    modifier onlyAdmin() {
        if (msg.sender != admin) revert NotAuthorized();
        _;
    }

    function registerVersion(
        string calldata version,
        Bundle calldata b
    ) external onlyAdmin {
        bytes32 key = keccak256(bytes(version));

        if (bundles[key].postageStamp != address(0)) {
            revert VersionAlreadyRegistered();
        }

        if (
            b.postageStamp == address(0) ||
            b.redistribution == address(0) ||
            b.staking == address(0) ||
            b.priceOracle == address(0)
        ) revert ZeroAddress();

        bundles[key] = b;

        emit VersionRegistered(
            key,
            version,
            b.postageStamp,
            b.redistribution,
            b.staking,
            b.priceOracle
        );
    }

    function get(
        string calldata version
    ) external view returns (Bundle memory) {
        return bundles[keccak256(bytes(version))];
    }
}

The problem I see is that it doesnt give us much benefits from current system, as bee nodes would still need to point to new registry contract values and be bumped up to new version. Just on less places. As we already have automated this bumping to new versions this actually doesnt simplify things. The biggest hurdle is migration of state and change of logic, which can only be effectively solved with proxies afaik.

[awmacpherson]

@0xCardiE your approach would also make it easier to automate stake migrations via bee, since every version of bee now has a way to track previous versions of the contract deployment without explicitly embedding it. And those previous version records are immutable, which eliminates an attack vector compared with bundling a list of all previous versions in bee.

[0xCardiE]

Been worked on here. https://github.com/ethersphere/sc-registry/
ask for access if you want to check details.

[mfw78]

Been worked on here. https://github.com/ethersphere/sc-registry/ ask for access if you want to check details.

Having private repositories is antithetical to a healthy open-source ecosystem. Why can this not be made public?

[0xCardiE]

yeah, makes sense, was just made like this by default until we approve its ready for public. Will check with managers if its ok to go public and do that.

[mfw78]

yeah, makes sense, was just made like this by default until we approve its ready for public. Will check with managers if its ok to go public and do that.

I would encourage open development from the start, gives ability for collective thinking at the design / implementation phase, prior to a "here is everything", which essentially robs the product / software of the ability to take external input that may highlight something that is missed, or of value.