Skip to content
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.

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

Modular pay gateway + audit fixes #8

Merged
merged 18 commits into from
Oct 15, 2024
Merged

Conversation

kumaryash90
Copy link
Member

No description provided.

@kumaryash90 kumaryash90 requested a review from jakeloo July 30, 2024 22:57
@kumaryash90 kumaryash90 marked this pull request as draft July 30, 2024 22:57
@kumaryash90 kumaryash90 requested a review from IDubuque July 31, 2024 17:55
@kumaryash90 kumaryash90 requested a review from GWSzeto August 1, 2024 20:30
keccak256("PayoutInfo(bytes32 clientId,address payoutAddress,uint256 feeAmount)");
bytes32 private constant REQUEST_TYPEHASH =
keccak256(
"PayRequest(bytes32 clientId,bytes32 transactionId,address tokenAddress,uint256 tokenAmount,uint256 expirationTimestamp,PayoutInfo[] payouts,address forwardAddress,bytes data)PayoutInfo(bytes32 clientId,address payoutAddress,uint256 feeAmount)"
Copy link

Choose a reason for hiding this comment

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

This might be a typo. I believe it should be just
PayRequest(bytes32 clientId,bytes32 transactionId,address tokenAddress,uint256 tokenAmount,uint256 expirationTimestamp,PayoutInfo[] payouts,address forwardAddress,bytes data)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this looks confusing, but we need nested structs to be represented this way in typehash.

EIP712 requires nested struct types to be sorted and appended to the encoding.

)
);

bytes32 digest = _hashTypedData(structHash);
Copy link

Choose a reason for hiding this comment

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

For gas saving purposes, we should probably implement the following function from solady EIP-712.
https://github.com/Vectorized/solady/blob/43f9d49815c8126d92771b26bd9bdbe2dbea87a5/src/utils/EIP712.sol#L95

Since this is a module, the domain separator will be initially signed with the module address in the constructor, but since this will be called via delegateCall the addresses won't match since it will be using the core contracts address

Solady's implementation cover's for this, but it's cheaper gas wise if we make it explicit:
https://github.com/Vectorized/solady/blob/43f9d49815c8126d92771b26bd9bdbe2dbea87a5/src/utils/EIP712.sol#L130

@kumaryash90 kumaryash90 marked this pull request as ready for review August 9, 2024 18:51
@kumaryash90 kumaryash90 force-pushed the yash/modular-pay-gateway branch from 04792c0 to d78ad75 Compare August 13, 2024 20:34
@kumaryash90 kumaryash90 changed the title Modular payments gateway Modular pay gateway Aug 13, 2024
@kumaryash90 kumaryash90 mentioned this pull request Aug 14, 2024
@kumaryash90 kumaryash90 changed the title Modular pay gateway Modular pay gateway + audit fixes Oct 15, 2024
@kumaryash90 kumaryash90 merged commit 618e1b2 into main Oct 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants