FilOzone / FilOzone/filecoin-pay

perf: `modifyRailPayment` and `modifyRailLockup` don't need to be `nonreentrant`

Open
#291 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Solidity
Stars
8
Forks
13
PR merge metrics
No merged PRs in 30d

Description

The only external call they do is `FVM.burn` (via `calculateAndPayFees`), which cannot lead to reentrancy.

Claude Sonnet 4.6 says:
```
The only external call sites in the entire contract are:
- FVMPay.burn (lines 1089, 1783)
- FVMPay.pay (line 778)
- token.safeTransfer (line 794)
- token.safeTransferFrom (line 802)
- IERC20Permit.permit (line 518)
- IValidator.validatePayment (line 441)
- IValidator.railTerminated (line 1434)
```

The functions that can reach these methods are:
```
FVMPay.burn (line 1089) — via calculateAndPayFees (native token only, non-zero amount):
- modifyRailPayment → processOneTimePayment → calculateAndPayFees
- settleRail → settleRailInternal → calculateAndPayFees
- settleTerminatedRailWithoutValidation → settleRailInternal → calculateAndPayFees

---
FVMPay.burn (line 1783) — directly in burnForFees:
- burnForFees

---
FVMPay.pay (line 778) — via withdrawToInternal (native token only):
- withdraw → withdrawToInternal
- withdrawTo → withdrawToInternal

---
token.safeTransfer (line 794) — via transferOut (ERC-20 only):
- withdraw → withdrawToInternal → transferOut
- withdrawTo → withdrawToInternal → transferOut
- burnForFees

---
token.safeTransferFrom (line 802) — via transferIn (ERC-20 only):
- deposit → transferIn
- depositWithPermit → _depositWithPermit → transferIn
- depositWithPermitAndApproveOperator → _depositWithPermit → transferIn
- depositWithPermitAndIncreaseOperatorApproval → _depositWithPermit → transferIn

---
IERC20Permit.permit (line 518) — via _depositWithPermit:
- depositWithPermit → _depositWithPermit
- depositWithPermitAndApproveOperator → _depositWithPermit
- depositWithPermitAndIncreaseOperatorApproval → _depositWithPermit

---
IValidator.validatePayment (line 1434)
- settleRail

---
IValidator.railTerminated (line 441)
- terminateRail
```

This analysis excluded `staticcall` like `balanceOf`, which are also not a reentrancy risk.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.