Skip to content

Commit 15dec1e

Browse files
committed
fix: address PR review feedback
- Fix typo "recwards" -> "rewards" in address-book CHANGELOG - Throw error instead of silent return on missing Arbiscan API key - Update SyncBytecodeDetectionFix docs to match actual implementation
1 parent b9bdfbe commit 15dec1e

3 files changed

Lines changed: 35 additions & 53 deletions

File tree

packages/address-book/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
### Minor Changes
66

7-
- Upgraded Rewards Manager and Subgraph Service with Rewards Eligibility Oracle and recwards reclaiming.
7+
- Upgraded Rewards Manager and Subgraph Service with Rewards Eligibility Oracle and rewards reclaiming.
88

99
## 1.1.0
1010

packages/deployment/docs/SyncBytecodeDetectionFix.md

Lines changed: 29 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,32 @@ The sync step saved the **implementation's bytecode** under the **proxy's deploy
4545

4646
## Fixes Applied
4747

48-
### Fix 1: Auto-Heal Bytecode Hash ([sync-utils.ts:641-683](../lib/sync-utils.ts#L641-L683))
48+
### Fix 1: Hash Comparison and Stale Record Cleanup ([sync-utils.ts:645-679](../lib/sync-utils.ts#L645-L679))
4949

50-
When sync detects missing/mismatched bytecode hash:
50+
When sync processes an implementation:
5151

52-
1. **Fetch on-chain bytecode** from the implementation address
53-
2. **Compare three versions**: local artifact, on-chain, address book
54-
3. **Auto-heal** if local matches on-chain:
52+
1. **Compare local artifact hash to address-book-stored hash**
53+
2. **If hashes match**: sync the implementation record to rocketh normally
54+
3. **If hashes don't match**: overwrite any stale rocketh record with empty bytecode, forcing a fresh deployment
5555

5656
```typescript
57-
if (localHash === onChainHash) {
58-
// Update address book with verified hash
59-
hashMatches = true
60-
shouldSync = true
61-
syncNotes.push('hash verified' or 'hash healed')
57+
if (storedHash && localHash) {
58+
hashMatches = storedHash === localHash
59+
}
60+
61+
// Clean up stale rocketh record if hash doesn't match
62+
if (!hashMatches && existingImpl) {
63+
// Overwrite stale record with empty bytecode - forces fresh deployment
64+
await env.save(`${spec.name}_Implementation`, {
65+
address: existingImpl.address,
66+
bytecode: '0x',
67+
deployedBytecode: undefined,
68+
...
69+
})
6270
}
6371
```
6472

65-
4. **Show clear status** if they differ:
66-
- `local code changed` - local differs from on-chain (ready to deploy)
67-
- `impl state unclear` - all three hashes differ (investigation needed)
68-
- `impl unverified` - couldn't fetch on-chain bytecode
73+
This ensures rocketh correctly detects when local code has changed and triggers a new deployment.
6974

7075
### Fix 2: Don't Store Wrong Bytecode for Proxy ([sync-utils.ts:508-532](../lib/sync-utils.ts#L508-L532))
7176

@@ -85,23 +90,10 @@ This ensures rocketh only uses implementation bytecode for the actual implementa
8590

8691
## Expected Behavior After Fix
8792

88-
### Scenario 1: Local Matches On-Chain (Hash Missing)
89-
90-
**Before**:
93+
### Scenario 1: Local Matches Address Book
9194

92-
```
93-
△ SubgraphService @ 0xc24A3dAC... → 0x2af1b0ed... (code changed)
94-
✓ SubgraphService implementation unchanged ← WRONG!
95-
```
96-
97-
**After**:
98-
99-
```
100-
△ SubgraphService @ 0xc24A3dAC... → 0x2af1b0ed... (hash verified)
101-
✓ SubgraphService implementation unchanged ← Correct (hash now matches)
102-
```
103-
104-
Address book is auto-healed with correct bytecode hash.
95+
When local artifact hash matches the stored hash, sync proceeds normally and rocketh
96+
correctly reports the implementation as unchanged.
10597

10698
### Scenario 2: Local Code Changed
10799

@@ -122,21 +114,11 @@ Address book is auto-healed with correct bytecode hash.
122114

123115
Deploy correctly detects the change and deploys new implementation.
124116

125-
### Scenario 3: Complex State (All Different)
126-
127-
**Before**:
128-
129-
```
130-
△ SubgraphService @ 0xc24A3dAC... → 0x2af1b0ed... (code changed)
131-
```
132-
133-
**After**:
134-
135-
```
136-
△ SubgraphService @ 0xc24A3dAC... → 0x2af1b0ed... (impl state unclear)
137-
```
117+
### Scenario 3: Stale Rocketh Record
138118

139-
Clear warning that investigation needed - all three hashes differ.
119+
When the hash doesn't match and a stale rocketh record exists, sync overwrites it
120+
with empty bytecode. This forces the next deploy to create a fresh implementation
121+
record rather than incorrectly reporting "unchanged".
140122

141123
## Testing
142124

@@ -156,10 +138,9 @@ npx hardhat deploy --skip-prompts --network arbitrumSepolia --tags subgraph-serv
156138

157139
## Migration Notes
158140

159-
- **No manual migration needed** - the fix auto-heals address books
160-
- First sync after fix will fetch on-chain bytecode and update hashes
161-
- Address book will be updated in place with correct metadata
162-
- Subsequent syncs will use the healed hashes
141+
- **No manual migration needed** - stale rocketh records are cleaned up automatically
142+
- First sync after fix will detect hash mismatches and clear stale records
143+
- Subsequent deploys will create fresh implementation records
163144

164145
## Related Files
165146

packages/deployment/tasks/verify-contract.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -534,10 +534,11 @@ const action: NewTaskActionFunction<TaskArgs> = async (taskArgs, hre) => {
534534
// Get API key from keystore
535535
const apiKey = await resolveConfigVar(hre, 'ARBISCAN_API_KEY')
536536
if (!apiKey) {
537-
console.error('\nError: No Arbiscan API key configured.')
538-
console.error('Set via keystore: npx hardhat keystore set ARBISCAN_API_KEY')
539-
console.error('Or environment: export ARBISCAN_API_KEY=...\n')
540-
return
537+
throw new Error(
538+
'No Arbiscan API key configured.\n' +
539+
'Set via keystore: npx hardhat keystore set ARBISCAN_API_KEY\n' +
540+
'Or environment: export ARBISCAN_API_KEY=...',
541+
)
541542
}
542543

543544
// Determine contracts to verify

0 commit comments

Comments
 (0)