`updateBAppsTokens` unintentionally sets newly added token risk levels to 0

[0xKorok]

Description

When a bApp adds new tokens via updateBAppsTokens(), these tokens are assigned a risk level of 0 instead of the specified risk level. The specified risk level is then stored as a pending value that only takes effect if the bApp owner calls updateBAppsTokens() again after waiting for the 14-day timelock period to expire.

Root Cause

In BasedAppsManager::updateBAppsTokens(), the logic treats updates that add a new token(s) and those which update existing token configurations identically:

uint32 requestTime = uint32(block.timestamp);

address token;
ICore.SharedRiskLevel storage tokenData;
ProtocolStorageLib.Data storage sp = ProtocolStorageLib.load();

for (uint256 i = 0; i < tokenConfigs.length; ) {
    token = tokenConfigs[i].token;
    tokenData = s.bAppTokens[msg.sender][token]; // <--- for a new token mapping returns default values 
    if (requestTime > tokenData.effectTime) {
        tokenData.currentValue = tokenData.pendingValue; // <---- currentValue set to 0
    }
    tokenData.pendingValue = tokenConfigs[i].sharedRiskLevel;  // <--- Sets intended risk as pending,
    tokenData.effectTime = requestTime + sp.tokenUpdateTimelockPeriod; // <--- Not adjustedable for 14 days
    tokenData.isSet = true; // <--- strategies can immediately make obligations
    unchecked {
        i++;
    }
}

For new tokens, both effectTime and pendingValue start at 0, so the condition requestTime > tokenData.effectTime evaluates to true, setting currentValue to 0 instead of the specified risk level.

Impact

• Strategies: Can immediately allocate to new tokens with risk level 0 instead of the intended risk level, potentially allowing unintended over-commitment behavior
• bApps: Weight calculations use incorrect risk parameters for 14+ days
• Protocol: Inconsistent behavior compared to initial token registration

Proof of Concept

1. Add the following test case to your existing BasedAppsManager.t.sol
2. Run forge test --match-contract BasedAppsManagerTest --match-test testNewTokenHasZeroRiskLevel -vvvv

---

function testNewTokenHasZeroRiskLevel() public {
    // 1. Register bApp with initial token
    ICore.TokenConfig[] memory initialTokens = createSingleTokenConfig(
        address(erc20mock), 
        1000000  // β = 1.0
    );
    vm.prank(USER1);
    proxiedManager.registerBApp(initialTokens, metadataURI);

    // 2. Add NEW token via updateBAppsTokens with intended risk level
    ICore.TokenConfig[] memory newTokens = createSingleTokenConfig(
        address(erc20mock2), 
        2000000  // β = 2.0 (INTENDED)
    );
    
    vm.prank(USER1);
    proxiedManager.updateBAppsTokens(newTokens);

    // 3. Demonstate that new token has wrong risk level
    (uint32 currentRisk, bool isSet, uint32 pendingRisk, ) = 
        proxiedManager.bAppTokens(USER1, address(erc20mock2));

    assertEq(isSet, true, "New token should be available immediately");
    assertEq(currentRisk, 0, "Issue: New token has risk level 0");
    assertEq(pendingRisk, 2000000, "Specified risk level is only pending");
    
    // bApp is now stuck with risk level 0 for 14 days
}

Proposed Solution

Distinguish between new and existing tokens in updateBAppsTokens():

if (!tokenData.isSet) {
    // NEW TOKEN: Set immediately (like registration)
    tokenData.currentValue = tokenConfigs[i].sharedRiskLevel;
    tokenData.isSet = true;
} else {
    // EXISTING TOKEN: Use timelock for changes
    if (requestTime > tokenData.effectTime) {
        tokenData.currentValue = tokenData.pendingValue;
    }
    tokenData.pendingValue = tokenConfigs[i].sharedRiskLevel;
    tokenData.effectTime = requestTime + sp.tokenUpdateTimelockPeriod;
}

This approach:

• Gives new tokens immediate effect with specified parameters
• Maintains timelock protection for existing token changes
• Eliminates the incorrect 0 assignment
• Creates consistent behavior with initial registration

Additional Context

The current logic creates inconsistent behavior in the protocol:

• registerBApp(): New tokens get immediate effect with specified risk levels
• updateBAppsTokens(): New tokens get incorrect 0 risk level for 14+ days

The immediate effect approach is more appropriate because:

1. Adding new tokens doesn't impact existing participants or their obligations
2. Strategies must voluntarily choose to allocate to new tokens
3. No existing harm occurs, unlike changing parameters of active tokens
4. bApp owners should have control over their own token parameters without unnecessary delays
5. The timelock mechanism should protect against changes to existing token parameters, not the addition of entirely new tokens

[0xKorok]

@mtabasco @riccardo-ssvlabs I’m curious to hear your thoughts on this when you have time. Thanks!

My understanding is that the purpose of sharedRiskLevels is to let the bApp express its risk preferences to attract strategies that match those preferences. If true the current behavior can’t be the intended behavior.

Also is this repo still a priority or has SSV focus shifted to a different approach?