Skip to content

[Fil 1232] updated validations for transfer method in Client SC #44

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

Open
RafalMagrys wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[Fil 1232] updated validations for transfer method in Client SC #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
112f6fe
temp
RafalMagrys Dec 11, 2025
67df9e1
test
RafalMagrys Dec 12, 2025
b77b3ed
abis
RafalMagrys Dec 12, 2025
a4ca39f
solhint
RafalMagrys Dec 12, 2025
dcea245
build and coverage fix
RafalMagrys Dec 12, 2025
3f8dd9c
removed casting to int
RafalMagrys Dec 12, 2025
c0d2f77
abis
RafalMagrys Dec 12, 2025
677e78f
cr fixes
RafalMagrys Dec 15, 2025
1b780d2
Yul exception
RafalMagrys Jan 2, 2026
71d8ecc
cr fixes
RafalMagrys Jan 2, 2026
22bd963
fmt fix
RafalMagrys Jan 2, 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
21 changes: 21 additions & 0 deletions abis/Client.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,27 @@
"name": "InsufficientAllowance",
"inputs": []
},
{
"type": "error",
"name": "InsufficientBeneficiaryAllocationExpiration",
"inputs": [
{
"name": "provider",
"type": "uint64",
"internalType": "CommonTypes.FilActorId"
},
{
"name": "beneficiaryExpiration",
"type": "int64",
"internalType": "int64"
},
{
"name": "requiredExpiration",
"type": "int64",
"internalType": "int64"
}
]
},
{
"type": "error",
"name": "InvalidActorID",
Expand Down
144 changes: 98 additions & 46 deletions src/Client.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {UtilsHandlers} from "filecoin-solidity/v0.8/utils/UtilsHandlers.sol";
import {MinerUtils} from "./libs/MinerUtils.sol";
import {BeneficiaryFactory} from "./BeneficiaryFactory.sol";
import {AllocationResponseCbor} from "./libs/AllocationResponseCbor.sol";
import {MinerTypes} from "filecoin-solidity/v0.8/types/MinerTypes.sol";

/**
* @title Client
Expand Down Expand Up @@ -162,9 +163,17 @@ contract Client is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
*/
error AllocationNotFound(CommonTypes.FilActorId provider, address client, uint64 allocationId);

/**
* @notice Thrown if beneficiary allocation expiration is insufficient
*/
error InsufficientBeneficiaryAllocationExpiration(
CommonTypes.FilActorId provider, int64 beneficiaryExpiration, int64 requiredExpiration
);

struct ProviderAllocation {
CommonTypes.FilActorId provider;
uint64 size;
int64 allocationTime;
Copy link
Copy Markdown
Collaborator

@Filip-L Filip-L Jan 2, 2026

Choose a reason for hiding this comment

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

Do we really need to store allocationTime for every allocation? I think this might be unnecessarily gas-consuming. Maybe we could move it to a separate struct or use a different approach.

Copy link
Copy Markdown
Collaborator Author

@RafalMagrys RafalMagrys Jan 2, 2026

Choose a reason for hiding this comment

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

I moved it to a new struct

}

struct ProviderClaim {
Expand Down Expand Up @@ -259,9 +268,13 @@ contract Client is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
if (failed) revert InvalidAmount();
uint256 datacapAmount = tokenAmount / TOKEN_PRECISION;

(ProviderAllocation[] memory allocations, ProviderClaim[] memory claimExtensions) =
_deserializeVerifregOperatorData(params.operator_data);
(
ProviderAllocation[] memory allocations,
ProviderClaim[] memory claimExtensions,
ProviderAllocation memory longestAllocation
) = _deserializeVerifregOperatorData(params.operator_data);

_verifyBeneficiaryExpiration(longestAllocation);
_verifyAndRegisterAllocations(allocations);
_verifyAndRegisterClaimExtensions(claimExtensions);
emit DatacapSpent(msg.sender, datacapAmount);
Expand Down Expand Up @@ -364,70 +377,109 @@ contract Client is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
}
}

/**
* @notice Verifies that the beneficiary expiration is sufficient for the longest allocation.
* @param longestAllocation The longest allocation.
* @dev Reverts with InsufficientBeneficiaryExpiration if the beneficiary expiration is insufficient.
*/
function _verifyBeneficiaryExpiration(ProviderAllocation memory longestAllocation) internal view {
if (longestAllocation.allocationTime != 0) {
MinerTypes.GetBeneficiaryReturn memory beneficiary =
MinerUtils.getBeneficiaryWithChecks(longestAllocation.provider, beneficiaryFactory, true, true, true);
Copy link
Copy Markdown
Collaborator

@Filip-L Filip-L Jan 2, 2026

Choose a reason for hiding this comment

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

getBeneficiaryWithChecks is used in _verifyAndRegisterAllocations, which might be the best place to validate the expiration. Within a single transfer, there can be more than one provider, each with a different beneficiary expiration time, but the check is only for one provider.

Copy link
Copy Markdown
Collaborator Author

@RafalMagrys RafalMagrys Jan 2, 2026

Choose a reason for hiding this comment

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

I can surely move it here, but why should I check everyone when in the task it is written to check only Miner’s beneficiary related to this longest allocation?

int64 beneficiaryExpiration = CommonTypes.ChainEpoch.unwrap(beneficiary.active.term.expiration);

if (longestAllocation.allocationTime > beneficiaryExpiration + 180 days) {
Copy link
Copy Markdown
Collaborator

@Filip-L Filip-L Jan 2, 2026

Choose a reason for hiding this comment

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

The beneficiary must expire no earlier than 180 days after the allocation ends.
longestAllocation.allocationTime + 180 days > beneficiaryExpiration

revert InsufficientBeneficiaryAllocationExpiration(
longestAllocation.provider, beneficiaryExpiration, longestAllocation.allocationTime
);
}

return;
}
}

// solhint-disable function-max-lines
/**
* @notice Deserialize Verifreg Operator Data.
* @param cborData The cbor encoded operator data.
* @return allocations Array of provider allocations.
* @return claimExtensions Array of provider claims.
* @return longestAllocation Allocation with the longest term.
*/
function _deserializeVerifregOperatorData(bytes memory cborData)
internal
pure
returns (ProviderAllocation[] memory allocations, ProviderClaim[] memory claimExtensions)
returns (
ProviderAllocation[] memory allocations,
ProviderClaim[] memory claimExtensions,
ProviderAllocation memory longestAllocation
)
{
uint256 operatorDataLength;
uint256 allocationRequestsLength;
uint256 claimExtensionRequestsLength;
uint256 resultLength;
Copy link
Copy Markdown
Collaborator

@Filip-L Filip-L Jan 2, 2026

Choose a reason for hiding this comment

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

Why did you change the names of these variables? The previous ones were more readable. Was this due to Solhint or the compiler?

Copy link
Copy Markdown
Collaborator

@kacperzuk-neti kacperzuk-neti Jan 2, 2026

Choose a reason for hiding this comment

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

Minimizing number of local variables to fit in the 15 slot stack depth limit of EVM.

uint64 provider;
uint64 claimId;
uint64 size;
uint256 byteIdx = 0;

(operatorDataLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
if (operatorDataLength != 2) revert InvalidOperatorData();

(allocationRequestsLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
allocations = new ProviderAllocation[](allocationRequestsLength);
for (uint256 i = 0; i < allocationRequestsLength; i++) {
uint256 allocationRequestLength;
(allocationRequestLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
(resultLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
if (resultLength != 2) revert InvalidOperatorData();

{
uint64 size;
int64 termMax;
int64 expiration;
(resultLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
allocations = new ProviderAllocation[](resultLength);
for (uint256 i = 0; i < resultLength; i++) {
uint256 allocationRequestLength;
(allocationRequestLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);

if (allocationRequestLength != 6) {
revert InvalidAllocationRequest();
}

if (allocationRequestLength != 6) {
revert InvalidAllocationRequest();
{
(provider, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
allocations[i].provider = CommonTypes.FilActorId.wrap(provider);
}
// slither-disable-start unused-return
(, byteIdx) = CBORDecoder.readBytes(cborData, byteIdx); // data (CID)
{
(size, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
allocations[i].size = size;
}
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx); // termMin
// slither-disable-end unused-return
{
(termMax, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx);
(expiration, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx);
allocations[i].allocationTime = termMax + expiration;

if (allocations[i].allocationTime > longestAllocation.allocationTime) {
longestAllocation = allocations[i];
}
}
}

(provider, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
// slither-disable-start unused-return
(, byteIdx) = CBORDecoder.readBytes(cborData, byteIdx); // data (CID)
(size, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx); // termMin
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx); // termMax
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx); // expiration
// slither-disable-end unused-return

allocations[i].provider = CommonTypes.FilActorId.wrap(provider);
allocations[i].size = size;
}
{
uint64 claimId;
(resultLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
claimExtensions = new ProviderClaim[](resultLength);
for (uint256 i = 0; i < resultLength; i++) {
uint256 claimExtensionRequestLength;
(claimExtensionRequestLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);

if (claimExtensionRequestLength != 3) {
revert InvalidClaimExtensionRequest();
}

(claimExtensionRequestsLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
claimExtensions = new ProviderClaim[](claimExtensionRequestsLength);
for (uint256 i = 0; i < claimExtensionRequestsLength; i++) {
uint256 claimExtensionRequestLength;
(claimExtensionRequestLength, byteIdx) = CBORDecoder.readFixedArray(cborData, byteIdx);
(provider, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
(claimId, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
// slither-disable-start unused-return
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx);
// slither-disable-end unused-return

if (claimExtensionRequestLength != 3) {
revert InvalidClaimExtensionRequest();
claimExtensions[i].provider = CommonTypes.FilActorId.wrap(provider);
claimExtensions[i].claim = CommonTypes.FilActorId.wrap(claimId);
}

(provider, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
(claimId, byteIdx) = CBORDecoder.readUInt64(cborData, byteIdx);
// slither-disable-start unused-return
(, byteIdx) = CBORDecoder.readInt64(cborData, byteIdx); // termMax
// slither-disable-end unused-return

claimExtensions[i].provider = CommonTypes.FilActorId.wrap(provider);
claimExtensions[i].claim = CommonTypes.FilActorId.wrap(claimId);
}
}

Expand Down
47 changes: 45 additions & 2 deletions test/Client.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ contract ClientTest is Test {
amount: CommonTypes.BigInt({
val: hex"010000000000000000000000000000000000000000000000000000000000000000", neg: false
}),
operator_data: hex"8282861903E8D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001A0007E9001A00503340190131861903E8D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001A0007E9001A0050334019013180"
operator_data: hex"8282861903E8D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001B00000078B30C40001A00503340190131861903E8D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001A0007E9001A0050334019013180"
});
vm.prank(clientAddress);
vm.expectRevert(abi.encodeWithSelector(Client.InvalidAmount.selector));
Expand Down Expand Up @@ -288,19 +288,28 @@ contract ClientTest is Test {
}

function testTransferRevertInsufficientAllowance() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));

vm.prank(clientAddress);
vm.expectRevert(abi.encodeWithSelector(Client.InsufficientAllowance.selector));
client.transfer(transferParams);
}

function testClientCanCallTransfer() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));

vm.prank(allocator);
client.increaseAllowance(clientAddress, SP2, 4096);
vm.prank(clientAddress);
client.transfer(transferParams);
}

function testVerifregFailIsDetected() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));

vm.prank(allocator);
client.increaseAllowance(clientAddress, SP2, 4096);
vm.etch(CALL_ACTOR_ID, address(builtInActorForTransferFunctionMock).code);
Expand All @@ -310,6 +319,9 @@ contract ClientTest is Test {
}

function testCheckAllowanceAfterTransfer() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));

vm.prank(allocator);
client.increaseAllowance(clientAddress, SP2, 4096);
vm.prank(clientAddress);
Expand All @@ -319,17 +331,20 @@ contract ClientTest is Test {

function testClaimExtensionNonExistent() public {
// 0 success_count
resolveAddress.setId(address(this), uint64(20000));
actorIdMock.setGetClaimsResult(hex"8282008080");
transferParams.operator_data = hex"82808183194E20011A005034AC";
vm.prank(clientAddress);
vm.expectRevert(Client.GetClaimsCallFailed.selector);
client.transfer(transferParams);
}

function testClaimExtensionn() public {
function testClaimExtension() public {
// params taken directly from `boost extend-deal` message
// no allocations
// 1 extension for provider 20000 and claim id 1
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));
vm.prank(allocator);
client.increaseAllowance(clientAddress, SP2, 4096);
transferParams.operator_data = hex"82808183194E20011A005034AC";
Expand Down Expand Up @@ -374,6 +389,8 @@ contract ClientTest is Test {
}

function testSPClientsMappingUpdateAllocations() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00c2a101", uint64(20000));
vm.prank(allocator);
client.increaseAllowance(clientAddress, SP2, 4096);

Expand Down Expand Up @@ -563,4 +580,30 @@ contract ClientTest is Test {
assertEq(CommonTypes.FilActorId.unwrap(ids[0]), 1);
assertEq(CommonTypes.FilActorId.unwrap(ids[1]), 2);
}

/**
* allocations: [{
* provider: 20000,
* expiration: 5250000000000,
* termMax: 1250000000000
* },{
* provider: 20000,
* expiration: 518400,
* termMax: 525
* }]
* claimExtensions: []
*/
function testTransferRevertInsufficientBeneficiaryExpiration() public {
resolveAddress.setId(address(this), uint64(20000));
resolveAddress.setAddress(hex"00C2A101", uint64(20000));
transferParams.operator_data =
hex"828286194E20D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001914401B000004C65C6294001B0000012309CE540086194E20D82A5828000181E203922020F2B9A58BBC9D9856E52EAB85155C1BA298F7E8DF458BD20A3AD767E11572CA221908001A0007E90019020D19013180";
vm.prank(clientAddress);
vm.expectRevert(
abi.encodeWithSelector(
Client.InsufficientBeneficiaryAllocationExpiration.selector, SP2, 6000000, 6500000000000
)
);
client.transfer(transferParams);
}
}