Skip to content

Get start time from parameter registry #254

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.

Sign up for GitHub

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

Draft
neekolas wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Get start time from parameter registry #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8af46e4
Get start time from parameter registry
neekolas Jan 27, 2026
a5e2483
Exclude reentrancy-balance detector from slither
neekolas Feb 3, 2026
9639eb4
Merge branch 'main' into 01-27-get_start_time_from_parameter_registry
KevinSmall Feb 4, 2026
5827a11
Update IRateRegistry.sol
neekolas Feb 4, 2026
e23e41d
Update IRateRegistry.sol
neekolas Feb 4, 2026
4f4c67c
Add start time validation to updateRates()
neekolas Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 50 additions & 50 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,52 @@
{
"name": "@xmtp/smart-contracts",
"version": "0.5.6",
"description": "XMTP Smart Contracts",
"author": "Ephemera Engineering Team <eng@ephemerahq.com>",
"repository": {
"type": "git",
"url": "git+https://github.com/xmtp/smart-contracts.git"
},
"bugs": {
"url": "https://github.com/xmtp/smart-contracts/issues"
},
"homepage": "https://github.com/xmtp/smart-contracts#readme",
"scripts": {
"build": "forge build",
"clean": "forge clean",
"coverage": "forge coverage --no-match-coverage '(script|test)' --gas-limit 3000000000 --report lcov && lcov --ignore-errors inconsistent --extract lcov.info --rc branch_coverage=1 --rc derive_function_end_line=0 -o lcov.info 'src/*' && genhtml --ignore-errors inconsistent lcov.info --rc branch_coverage=1 --rc derive_function_end_line=0 -o coverage",
"gas-report": "forge test --fuzz-seed 10101 --gas-report --gas-limit 3000000000 > gasreport.ansi",
"doc": "forge doc --serve --port 4000",
"lint-staged": "lint-staged",
"prettier": "prettier --write 'script/**/*' 'src/**/*' 'test/**/*' 'config/**/*' 'environments/**/*'",
"prettier-check": "prettier --check 'script/**/*' 'src/**/*' 'test/**/*' 'config/**/*' 'environments/**/*'",
"slither": "rm -rf output.sarif && forge build --build-info --skip '*/test/**' --skip '*/script/**' --force && slither --exclude low-level-calls --sarif output.sarif --compile-force-framework foundry --ignore-compile --config-file slither.config.json --fail-high .",
"solhint": "solhint -f stylish 'src/**/*.sol'",
"solhint-fix": "solhint --fix 'src/**/*.sol'",
"test": "forge test",
"anvil": "anvil",
"gen-anvil-state": "./dev/gen-anvil-state",
"gen-artifacts": "./dev/gen-artifacts",
"deploy-local": "./dev/deploy-local",
"check-deployments-local": "./dev/check-local-deployment"
},
"devDependencies": {
"@arbitrum/sdk": "^4.0.4",
"@ethersproject/bignumber": "^5.8.0",
"@fireblocks/fireblocks-json-rpc": "^0.2.2",
"dotenv": "^16.5.0",
"ethers": "5.8.0",
"lint-staged": "^15.5.2",
"prettier": "^3.5.3",
"prettier-plugin-solidity": "^1.4.3",
"solhint": "^5.1.0",
"solhint-plugin-prettier": "^0.1.0"
},
"engines": {
"node": ">=23"
},
"files": [
"src/**",
"out/**"
]
"name": "@xmtp/smart-contracts",
"version": "0.5.6",
"description": "XMTP Smart Contracts",
"author": "Ephemera Engineering Team <eng@ephemerahq.com>",
"repository": {
"type": "git",
"url": "git+https://github.com/xmtp/smart-contracts.git"
},
"bugs": {
"url": "https://github.com/xmtp/smart-contracts/issues"
},
"homepage": "https://github.com/xmtp/smart-contracts#readme",
"scripts": {
"build": "forge build",
"clean": "forge clean",
"coverage": "forge coverage --no-match-coverage '(script|test)' --gas-limit 3000000000 --report lcov && lcov --ignore-errors inconsistent --extract lcov.info --rc branch_coverage=1 --rc derive_function_end_line=0 -o lcov.info 'src/*' && genhtml --ignore-errors inconsistent lcov.info --rc branch_coverage=1 --rc derive_function_end_line=0 -o coverage",
"gas-report": "forge test --fuzz-seed 10101 --gas-report --gas-limit 3000000000 > gasreport.ansi",
"doc": "forge doc --serve --port 4000",
"lint-staged": "lint-staged",
"prettier": "prettier --write 'script/**/*' 'src/**/*' 'test/**/*' 'config/**/*' 'environments/**/*'",
"prettier-check": "prettier --check 'script/**/*' 'src/**/*' 'test/**/*' 'config/**/*' 'environments/**/*'",
"slither": "rm -rf output.sarif && forge build --build-info --skip '*/test/**' --skip '*/script/**' --force && slither --exclude low-level-calls --sarif output.sarif --compile-force-framework foundry --ignore-compile --config-file slither.config.json --fail-high .",
"solhint": "solhint -f stylish 'src/**/*.sol'",
"solhint-fix": "solhint --fix 'src/**/*.sol'",
"test": "forge test",
"anvil": "anvil",
"gen-anvil-state": "./dev/gen-anvil-state",
"gen-artifacts": "./dev/gen-artifacts",
"deploy-local": "./dev/deploy-local",
"check-deployments-local": "./dev/check-local-deployment"
},
"devDependencies": {
"@arbitrum/sdk": "^4.0.4",
"@ethersproject/bignumber": "^5.8.0",
"@fireblocks/fireblocks-json-rpc": "^0.2.2",
"dotenv": "^16.5.0",
"ethers": "5.8.0",
"lint-staged": "^15.5.2",
"prettier": "^3.5.3",
"prettier-plugin-solidity": "^1.4.3",
"solhint": "^5.1.0",
"solhint-plugin-prettier": "^0.1.0"
},
"engines": {
"node": ">=23"
},
"files": [
"src/**",
"out/**"
]
}
17 changes: 14 additions & 3 deletions script/DeployLocal.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ contract DeployLocalScripts is Script {
string internal constant _RATE_REGISTRY_STORAGE_FEE_KEY = "xmtp.rateRegistry.storageFee";
string internal constant _RATE_REGISTRY_CONGESTION_FEE_KEY = "xmtp.rateRegistry.congestionFee";
string internal constant _RATE_REGISTRY_TARGET_RATE_PER_MINUTE_KEY = "xmtp.rateRegistry.targetRatePerMinute";
string internal constant _RATE_REGISTRY_RATES_IN_EFFECT_AFTER_KEY = "xmtp.rateRegistry.ratesInEffectAfter";

string internal constant _NODE_REGISTRY_ADMIN_KEY = "xmtp.nodeRegistry.admin";
string internal constant _NODE_REGISTRY_MAX_CANONICAL_NODES_KEY = "xmtp.nodeRegistry.maxCanonicalNodes";
Expand Down Expand Up @@ -791,17 +792,19 @@ contract DeployLocalScripts is Script {
}

function _setRateRegistryStartingRates() internal {
string[] memory keys_ = new string[](4);
string[] memory keys_ = new string[](5);
keys_[0] = _RATE_REGISTRY_MESSAGE_FEE_KEY;
keys_[1] = _RATE_REGISTRY_STORAGE_FEE_KEY;
keys_[2] = _RATE_REGISTRY_CONGESTION_FEE_KEY;
keys_[3] = _RATE_REGISTRY_TARGET_RATE_PER_MINUTE_KEY;
keys_[4] = _RATE_REGISTRY_RATES_IN_EFFECT_AFTER_KEY;

bytes32[] memory values_ = new bytes32[](4);
bytes32[] memory values_ = new bytes32[](5);
values_[0] = bytes32(_RATE_REGISTRY_STARTING_MESSAGE_FEE);
values_[1] = bytes32(_RATE_REGISTRY_STARTING_STORAGE_FEE);
values_[2] = bytes32(_RATE_REGISTRY_STARTING_CONGESTION_FEE);
values_[3] = bytes32(_RATE_REGISTRY_STARTING_TARGET_RATE_PER_MINUTE);
values_[4] = bytes32(uint256(block.timestamp));

vm.startBroadcast(_privateKey);
_parameterRegistryProxy.set(keys_, values_);
Expand All @@ -817,6 +820,10 @@ contract DeployLocalScripts is Script {
if (_parameterRegistryProxy.get(keys_[3]) != values_[3]) {
revert("Rate registry target rate per minute not set correctly");
}

if (_parameterRegistryProxy.get(keys_[4]) != values_[4]) {
revert("Rate registry rates in effect after not set correctly");
}
}

function _updateRateRegistryRates() internal {
Expand All @@ -841,7 +848,11 @@ contract DeployLocalScripts is Script {
revert("Rate registry target rate per minute mismatch");
}

if (rates_[0].startTime != uint64(vm.getBlockTimestamp())) revert("Rate registry start time mismatch");
uint64 expectedStartTime_ = uint64(
uint256(_parameterRegistryProxy.get(_RATE_REGISTRY_RATES_IN_EFFECT_AFTER_KEY))
);

if (rates_[0].startTime != expectedStartTime_) revert("Rate registry start time mismatch");
}

/* ============ Node Registry Helpers ============ */
Expand Down
2 changes: 1 addition & 1 deletion slither.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"detectors_to_exclude": "",
"detectors_to_exclude": "reentrancy-balance",
Copy link
Collaborator

@KevinSmall KevinSmall Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to mention, we could drop this change if we wanted.
Another PR pinned slither in CI and so the CI error has gone away.

"filter_paths": "lib"
}
23 changes: 20 additions & 3 deletions src/settlement-chain/RateRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,14 @@ contract RateRegistry is IRateRegistry, Migratable, Initializable {
targetRatePerMinuteParameterKey()
);

_revertIfNoRateChange(messageFee_, storageFee_, congestionFee_, targetRatePerMinute_);
uint64 startTime_ = RegistryParameters.getUint64Parameter(parameterRegistry, ratesInEffectAfterParameterKey());
Copy link
Collaborator

@KevinSmall KevinSmall Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be overkill to add this in, but we could add validation here to see if startTime_ were set to something unexpected, like in the past?
If we can, I'd favor trusting our admin process to get it right, and keep contract simple.

Copy link
Collaborator Author

@neekolas neekolas Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one validation that seems the most important is that the start time of the new rates is after the start time of the previous rates. The nodes will reject out of order rate changes, but it would be nice to cut them off at the source.


uint64 startTime_ = uint64(block.timestamp);
_revertIfNoRateChange(messageFee_, storageFee_, congestionFee_, targetRatePerMinute_);
Copy link
Collaborator

@KevinSmall KevinSmall Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want a change in just the startTime_ to count as a valid change? At present if we changed just the startTime_ it'd count as "no rate change" here, and the transaction would revert. Which might be fine.

Copy link
Collaborator Author

@neekolas neekolas Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. I initially had it in the calculation and Octane made some very good points.

The concern is that if the startTime moved ahead of other parameters and updateRates got called you could end up with duplicate rates (old parameters new start time). And then when it did get called again you would end up with two rates that have the same start time.

_revertIfInvalidStartTime(startTime_);

$.allRates.push(Rates(messageFee_, storageFee_, congestionFee_, targetRatePerMinute_, startTime_));

emit RatesUpdated(messageFee_, storageFee_, congestionFee_, targetRatePerMinute_);
emit RatesUpdated(messageFee_, storageFee_, congestionFee_, targetRatePerMinute_, startTime_);
}

/// @inheritdoc IMigratable
Expand Down Expand Up @@ -117,6 +118,11 @@ contract RateRegistry is IRateRegistry, Migratable, Initializable {
return "xmtp.rateRegistry.targetRatePerMinute";
}

/// @inheritdoc IRateRegistry
function ratesInEffectAfterParameterKey() public pure returns (string memory key_) {
return "xmtp.rateRegistry.ratesInEffectAfter";
}

/// @inheritdoc IRateRegistry
function migratorParameterKey() public pure returns (string memory key_) {
return "xmtp.rateRegistry.migrator";
Expand Down Expand Up @@ -181,4 +187,15 @@ contract RateRegistry is IRateRegistry, Migratable, Initializable {
revert NoChange();
}
}

/// @dev Reverts if the start time is not strictly greater than the last start time.
function _revertIfInvalidStartTime(uint64 startTime_) internal view {
RateRegistryStorage storage $ = _getRateRegistryStorage();

if ($.allRates.length == 0) return;

uint64 lastStartTime_ = $.allRates[$.allRates.length - 1].startTime;

if (startTime_ <= lastStartTime_) revert InvalidStartTime(startTime_, lastStartTime_);
}
}
15 changes: 14 additions & 1 deletion src/settlement-chain/interfaces/IRateRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,15 @@ interface IRateRegistry is IMigratable, IIdentified, IRegistryParametersErrors {
* @param storageFee The storage fee.
* @param congestionFee The congestion fee.
* @param targetRatePerMinute The target rate per minute.
* @param startTime The start time of the rate.
*/
event RatesUpdated(uint64 messageFee, uint64 storageFee, uint64 congestionFee, uint64 targetRatePerMinute);
event RatesUpdated(
uint64 messageFee,
uint64 storageFee,
uint64 congestionFee,
uint64 targetRatePerMinute,
uint64 startTime
);

/* ============ Custom Errors ============ */

Expand All @@ -56,6 +63,9 @@ interface IRateRegistry is IMigratable, IIdentified, IRegistryParametersErrors {
/// @notice Thrown when there is no change to an updated parameter.
error NoChange();

/// @notice Thrown when the new start time is not strictly greater than the last start time.
error InvalidStartTime(uint64 startTime, uint64 lastStartTime);

/* ============ Initialization ============ */

/**
Expand Down Expand Up @@ -95,6 +105,9 @@ interface IRateRegistry is IMigratable, IIdentified, IRegistryParametersErrors {
/// @notice The parameter registry key used to fetch the target rate per minute.
function targetRatePerMinuteParameterKey() external pure returns (string memory key_);

/// @notice The parameter registry key used to fetch the rates in effect after timestamp.
function ratesInEffectAfterParameterKey() external pure returns (string memory key_);

/// @notice The parameter registry key used to fetch the migrator.
function migratorParameterKey() external pure returns (string memory key_);

Expand Down
Loading
Loading