Skip to content

Commit 0cd7aee

Browse files
Antoine de Chevignéclaude
Antoine de Chevigné
and
claude
committed
Address additional code review feedback for OP Stack
- Fix computeBlobVersionedHash slicing (handle 0x prefix correctly) - Add node-fetch import for beacon API calls - Add try/catch error handling to all parseLog functions - Add lowercase normalization setters for all address fields in OpChainConfig - Fix safeUpdate to only validate parentChainId when provided 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 4790c14 commit 0cd7aee

File tree

5 files changed

+119
-67
lines changed

5 files changed

+119
-67
lines changed

‎run/lib/opBatches.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const { ethers } = require('ethers');
2+
const fetch = require('node-fetch');
23

34
/**
45
* OP Stack Batch Parsing Library
@@ -138,8 +139,10 @@ const computeBlobVersionedHash = (kzgCommitment) => {
138139
const commitment = kzgCommitment.startsWith('0x') ? kzgCommitment : '0x' + kzgCommitment;
139140
// EIP-4844 uses SHA-256 for versioned hashes, not keccak256
140141
const hash = ethers.utils.sha256(commitment);
141-
// Version 0x01 for KZG commitments
142-
return '0x01' + hash.slice(4);
142+
// Remove 0x prefix if present, then prepend version byte 0x01
143+
const hashWithoutPrefix = hash.startsWith('0x') ? hash.slice(2) : hash;
144+
// Version 0x01 for KZG commitments, result is 0x01 + 31 bytes of hash = 66 chars total
145+
return '0x01' + hashWithoutPrefix.slice(0, 62);
143146
};
144147

145148
/**

‎run/lib/opDeposits.js‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,26 @@ const parseOpaqueData = (opaqueData) => {
5353
/**
5454
* Get deposit data from a TransactionDeposited log
5555
* @param {Object} log - The log to parse
56-
* @returns {Object} Parsed deposit data
56+
* @returns {Object|null} Parsed deposit data or null if parsing fails
5757
*/
5858
const getTransactionDepositedData = (log) => {
59-
const parsedLog = iface.parseLog({ topics: log.topics, data: log.data });
60-
const opaqueData = parseOpaqueData(parsedLog.args.opaqueData);
59+
try {
60+
const parsedLog = iface.parseLog({ topics: log.topics, data: log.data });
61+
const opaqueData = parseOpaqueData(parsedLog.args.opaqueData);
6162

62-
return {
63-
from: parsedLog.args.from.toLowerCase(),
64-
to: parsedLog.args.to.toLowerCase(),
65-
version: parsedLog.args.version.toString(),
66-
value: opaqueData.value,
67-
gasLimit: opaqueData.gasLimit,
68-
isCreation: opaqueData.isCreation,
69-
data: opaqueData.data
70-
};
63+
return {
64+
from: parsedLog.args.from.toLowerCase(),
65+
to: parsedLog.args.to.toLowerCase(),
66+
version: parsedLog.args.version.toString(),
67+
value: opaqueData.value,
68+
gasLimit: opaqueData.gasLimit,
69+
isCreation: opaqueData.isCreation,
70+
data: opaqueData.data
71+
};
72+
} catch (error) {
73+
console.error('Error parsing TransactionDeposited log:', error.message);
74+
return null;
75+
}
7176
};
7277

7378
/**

‎run/lib/opOutputs.js‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,36 +37,46 @@ const isDisputeGameCreatedLog = (log) => {
3737
/**
3838
* Get data from an OutputProposed log (legacy)
3939
* @param {Object} log - The log to parse
40-
* @returns {Object} Parsed output data
40+
* @returns {Object|null} Parsed output data or null if parsing fails
4141
*/
4242
const getOutputProposedData = (log) => {
43-
const parsedLog = l2OutputOracleIface.parseLog({ topics: log.topics, data: log.data });
43+
try {
44+
const parsedLog = l2OutputOracleIface.parseLog({ topics: log.topics, data: log.data });
4445

45-
return {
46-
outputRoot: parsedLog.args.outputRoot,
47-
l2OutputIndex: parsedLog.args.l2OutputIndex.toString(),
48-
l2BlockNumber: parsedLog.args.l2BlockNumber.toString(),
49-
l1Timestamp: parsedLog.args.l1Timestamp.toString()
50-
};
46+
return {
47+
outputRoot: parsedLog.args.outputRoot,
48+
l2OutputIndex: parsedLog.args.l2OutputIndex.toString(),
49+
l2BlockNumber: parsedLog.args.l2BlockNumber.toString(),
50+
l1Timestamp: parsedLog.args.l1Timestamp.toString()
51+
};
52+
} catch (error) {
53+
console.error('Error parsing OutputProposed log:', error.message);
54+
return null;
55+
}
5156
};
5257

5358
/**
5459
* Get data from a DisputeGameCreated log (modern/fault proofs)
5560
* @param {Object} log - The log to parse
56-
* @returns {Object} Parsed dispute game data
61+
* @returns {Object|null} Parsed dispute game data or null if parsing fails
5762
*/
5863
const getDisputeGameCreatedData = (log) => {
59-
const parsedLog = disputeGameFactoryIface.parseLog({ topics: log.topics, data: log.data });
60-
const rawGameType = parsedLog.args.gameType;
64+
try {
65+
const parsedLog = disputeGameFactoryIface.parseLog({ topics: log.topics, data: log.data });
66+
const rawGameType = parsedLog.args.gameType;
6167

62-
return {
63-
disputeProxy: parsedLog.args.disputeProxy.toLowerCase(),
64-
// Convert BigNumber to primitive number for DB storage and JSON serialization
65-
gameType: typeof rawGameType === 'object' && rawGameType !== null && typeof rawGameType.toNumber === 'function'
66-
? rawGameType.toNumber()
67-
: Number(rawGameType),
68-
rootClaim: parsedLog.args.rootClaim
69-
};
68+
return {
69+
disputeProxy: parsedLog.args.disputeProxy.toLowerCase(),
70+
// Convert BigNumber to primitive number for DB storage and JSON serialization
71+
gameType: typeof rawGameType === 'object' && rawGameType !== null && typeof rawGameType.toNumber === 'function'
72+
? rawGameType.toNumber()
73+
: Number(rawGameType),
74+
rootClaim: parsedLog.args.rootClaim
75+
};
76+
} catch (error) {
77+
console.error('Error parsing DisputeGameCreated log:', error.message);
78+
return null;
79+
}
7080
};
7181

7282
/**

‎run/lib/opWithdrawals.js‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,49 +54,64 @@ const isWithdrawalFinalizedLog = (log) => {
5454
/**
5555
* Get data from a MessagePassed log (L2 withdrawal initiation)
5656
* @param {Object} log - The log to parse
57-
* @returns {Object} Parsed withdrawal initiation data
57+
* @returns {Object|null} Parsed withdrawal initiation data or null if parsing fails
5858
*/
5959
const getMessagePassedData = (log) => {
60-
const parsedLog = l2ToL1MessagePasserIface.parseLog({ topics: log.topics, data: log.data });
60+
try {
61+
const parsedLog = l2ToL1MessagePasserIface.parseLog({ topics: log.topics, data: log.data });
6162

62-
return {
63-
nonce: parsedLog.args.nonce.toString(),
64-
sender: parsedLog.args.sender.toLowerCase(),
65-
target: parsedLog.args.target.toLowerCase(),
66-
value: parsedLog.args.value.toString(),
67-
gasLimit: parsedLog.args.gasLimit.toString(),
68-
data: parsedLog.args.data,
69-
withdrawalHash: parsedLog.args.withdrawalHash
70-
};
63+
return {
64+
nonce: parsedLog.args.nonce.toString(),
65+
sender: parsedLog.args.sender.toLowerCase(),
66+
target: parsedLog.args.target.toLowerCase(),
67+
value: parsedLog.args.value.toString(),
68+
gasLimit: parsedLog.args.gasLimit.toString(),
69+
data: parsedLog.args.data,
70+
withdrawalHash: parsedLog.args.withdrawalHash
71+
};
72+
} catch (error) {
73+
console.error('Error parsing MessagePassed log:', error.message);
74+
return null;
75+
}
7176
};
7277

7378
/**
7479
* Get data from a WithdrawalProven log (L1)
7580
* @param {Object} log - The log to parse
76-
* @returns {Object} Parsed withdrawal proven data
81+
* @returns {Object|null} Parsed withdrawal proven data or null if parsing fails
7782
*/
7883
const getWithdrawalProvenData = (log) => {
79-
const parsedLog = optimismPortalIface.parseLog({ topics: log.topics, data: log.data });
84+
try {
85+
const parsedLog = optimismPortalIface.parseLog({ topics: log.topics, data: log.data });
8086

81-
return {
82-
withdrawalHash: parsedLog.args.withdrawalHash,
83-
from: parsedLog.args.from.toLowerCase(),
84-
to: parsedLog.args.to.toLowerCase()
85-
};
87+
return {
88+
withdrawalHash: parsedLog.args.withdrawalHash,
89+
from: parsedLog.args.from.toLowerCase(),
90+
to: parsedLog.args.to.toLowerCase()
91+
};
92+
} catch (error) {
93+
console.error('Error parsing WithdrawalProven log:', error.message);
94+
return null;
95+
}
8696
};
8797

8898
/**
8999
* Get data from a WithdrawalFinalized log (L1)
90100
* @param {Object} log - The log to parse
91-
* @returns {Object} Parsed withdrawal finalized data
101+
* @returns {Object|null} Parsed withdrawal finalized data or null if parsing fails
92102
*/
93103
const getWithdrawalFinalizedData = (log) => {
94-
const parsedLog = optimismPortalIface.parseLog({ topics: log.topics, data: log.data });
104+
try {
105+
const parsedLog = optimismPortalIface.parseLog({ topics: log.topics, data: log.data });
95106

96-
return {
97-
withdrawalHash: parsedLog.args.withdrawalHash,
98-
success: parsedLog.args.success
99-
};
107+
return {
108+
withdrawalHash: parsedLog.args.withdrawalHash,
109+
success: parsedLog.args.success
110+
};
111+
} catch (error) {
112+
console.error('Error parsing WithdrawalFinalized log:', error.message);
113+
return null;
114+
}
100115
};
101116

102117
/**

‎run/models/opChainConfig.js‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,24 @@ module.exports = (sequelize, DataTypes) => {
2828
'parentChainExplorer'
2929
];
3030

31-
const supportedParentChains = await sequelize.models.Workspace.getAvailableTopOpParent();
32-
const supportedParentChainIds = supportedParentChains.map(chain => chain.networkId);
33-
if (!supportedParentChainIds.includes(params.parentChainId)) {
34-
throw new Error(`Parent chain network is not supported yet. Available networks: ${supportedParentChainIds.join(', ')}`);
35-
}
36-
3731
const filteredParams = {};
3832
for (const [key, value] of Object.entries(params)) {
3933
if (allowedParams.includes(key)) {
4034
filteredParams[key] = value;
4135
}
4236
}
4337

44-
return this.update({
45-
...filteredParams,
46-
parentWorkspaceId: supportedParentChains.find(chain => chain.networkId === params.parentChainId).id
47-
});
38+
// Only validate and update parentChainId if provided
39+
if (params.parentChainId !== undefined) {
40+
const supportedParentChains = await sequelize.models.Workspace.getAvailableTopOpParent();
41+
const supportedParentChainIds = supportedParentChains.map(chain => chain.networkId);
42+
if (!supportedParentChainIds.includes(params.parentChainId)) {
43+
throw new Error(`Parent chain network is not supported yet. Available networks: ${supportedParentChainIds.join(', ')}`);
44+
}
45+
filteredParams.parentWorkspaceId = supportedParentChains.find(chain => chain.networkId === params.parentChainId).id;
46+
}
47+
48+
return this.update(filteredParams);
4849
}
4950
}
5051

@@ -68,6 +69,9 @@ module.exports = (sequelize, DataTypes) => {
6869
batchInboxAddress: {
6970
type: DataTypes.STRING(42),
7071
allowNull: false,
72+
set(value) {
73+
this.setDataValue('batchInboxAddress', value ? value.toLowerCase() : value);
74+
},
7175
validate: {
7276
isEthereumAddress(value) {
7377
if (!/^0x[a-fA-F0-9]{40}$/.test(value)) {
@@ -79,6 +83,9 @@ module.exports = (sequelize, DataTypes) => {
7983
optimismPortalAddress: {
8084
type: DataTypes.STRING(42),
8185
allowNull: false,
86+
set(value) {
87+
this.setDataValue('optimismPortalAddress', value ? value.toLowerCase() : value);
88+
},
8289
validate: {
8390
isEthereumAddress(value) {
8491
if (!/^0x[a-fA-F0-9]{40}$/.test(value)) {
@@ -90,6 +97,9 @@ module.exports = (sequelize, DataTypes) => {
9097
l2OutputOracleAddress: {
9198
type: DataTypes.STRING(42),
9299
allowNull: true,
100+
set(value) {
101+
this.setDataValue('l2OutputOracleAddress', value ? value.toLowerCase() : value);
102+
},
93103
validate: {
94104
isEthereumAddress(value) {
95105
if (value && !/^0x[a-fA-F0-9]{40}$/.test(value)) {
@@ -101,6 +111,9 @@ module.exports = (sequelize, DataTypes) => {
101111
disputeGameFactoryAddress: {
102112
type: DataTypes.STRING(42),
103113
allowNull: true,
114+
set(value) {
115+
this.setDataValue('disputeGameFactoryAddress', value ? value.toLowerCase() : value);
116+
},
104117
validate: {
105118
isEthereumAddress(value) {
106119
if (value && !/^0x[a-fA-F0-9]{40}$/.test(value)) {
@@ -112,6 +125,9 @@ module.exports = (sequelize, DataTypes) => {
112125
systemConfigAddress: {
113126
type: DataTypes.STRING(42),
114127
allowNull: true,
128+
set(value) {
129+
this.setDataValue('systemConfigAddress', value ? value.toLowerCase() : value);
130+
},
115131
validate: {
116132
isEthereumAddress(value) {
117133
if (value && !/^0x[a-fA-F0-9]{40}$/.test(value)) {
@@ -124,6 +140,9 @@ module.exports = (sequelize, DataTypes) => {
124140
type: DataTypes.STRING(42),
125141
allowNull: false,
126142
defaultValue: '0x4200000000000000000000000000000000000016',
143+
set(value) {
144+
this.setDataValue('l2ToL1MessagePasserAddress', value ? value.toLowerCase() : value);
145+
},
127146
validate: {
128147
isEthereumAddress(value) {
129148
if (!/^0x[a-fA-F0-9]{40}$/.test(value)) {

0 commit comments

Comments
 (0)