feat: add all data to deployment events #1487
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used📓 Learnings (3)packages/payment-processor/src/payment/single-request-proxy.ts (6)packages/payment-processor/test/payment/single-request-proxy.test.ts (6)packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/0.1.0.json (1)🔇 Additional comments (9)packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts (2)
The updated Sepolia deployment details reflect the new contract deployment required for the enhanced event signatures that include fee-related parameters.
Let's verify that the new contract deployment is properly set up: Please ensure:
packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/0.1.0.json (2)
The fee-related parameters mirror those added to the ERC20 event, maintaining a consistent structure across both event types. The non-indexed status of these parameters aligns with our gas optimization practices. Let's verify the event emission implementation: ✅ Verification successfulEvent implementation is correctly updated with fee parameters The event emission in 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the event emission includes all new parameters
# Expected: Find event emission with all parameters in the implementation
ast-grep --pattern 'emit EthereumSingleRequestProxyCreated($$$, feeAddress, feeAmount, feeProxyUsed)'
Length of output: 103 Script: #!/bin/bash
# Let's first find all files that might contain the event emission
fd -e sol -e js -e ts | xargs rg "EthereumSingleRequestProxyCreated" -B 2 -A 2
Length of output: 3143 Script: #!/bin/bash
# Let's check the actual event emission in the contract implementation
cat packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol
Length of output: 4406
The new fee-related parameters ( Let's verify the event emission implementation: ✅ Verification successfulEvent emissions correctly include all new fee-related parameters The implementation in 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the event emission includes all new parameters
# Expected: Find event emission with all parameters in the implementation
ast-grep --pattern 'emit ERC20SingleRequestProxyCreated($$$, feeAddress, feeAmount, feeProxyUsed)'
Length of output: 103 Script: #!/bin/bash
# Let's try to find the contract implementation file first
fd "SingleRequestProxyFactory" --type f --extension sol
# Then we'll search for the event emission in the implementation
rg "emit (ERC20|Ethereum)SingleRequestProxyCreated" -A 5
Length of output: 1298 Script: #!/bin/bash
# Let's get more context around the event emissions to see all parameters
rg "emit (ERC20|Ethereum)SingleRequestProxyCreated" -B 2 -A 8 packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol
Length of output: 559 packages/payment-processor/src/payment/single-request-proxy.ts (2)
The implementation safely extracts the proxy address using optional chaining with a fallback mechanism.
The event handling implementation is more reliable now, using explicit event type checking based on the proxy type. Let's verify the event names match the contract: ✅ Verification successfulEvent names match the contract implementation perfectly The verification confirms that both event names 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify event names in SingleRequestProxyFactory contract
# Expected: Events named 'ERC20SingleRequestProxyCreated' and 'EthereumSingleRequestProxyCreated'
rg -A 2 'event (ERC20|Ethereum)SingleRequestProxyCreated'
Length of output: 649 packages/payment-processor/test/payment/single-request-proxy.test.ts (3)
The change from event count to block-based filtering is a more reliable approach for event verification. Also applies to: 184-184
The event verification now properly checks all parameters including the fee-related data. The
The event verification follows the same pattern as the Ethereum proxy, with proper verification of all parameters including token address and fee-related data. The Also applies to: 219-219, 225-238 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
requested review from
alexandre-abrioux,
leoslr and
yomarion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
packages/smart-contracts/test/contracts/SingleRequestProxyFactory.test.ts (1)
Line range hint
1-217: Consider adding edge case tests for fee parameters.While the current test coverage is good, consider adding the following test cases to ensure robust validation of the fee-related functionality:
- Creation with zero fee amount
- Creation with zero address for fee recipient
- Creation when fee amount exceeds reasonable limits
Would you like me to provide example test implementations for these cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol(3 hunks)packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts(0 hunks)packages/smart-contracts/test/contracts/SingleRequestProxyFactory.test.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts
🧰 Additional context used
📓 Learnings (2)
packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol (3)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:0-0
Timestamp: 2024-09-27T11:42:01.062Z
Learning: In the `createEthereumSingleRequestProxy` function, it's acceptable for `_feeAddress` to be the zero address, as setting the fee address to zero is a common practice. Adding zero address checks is unnecessary and adds extra gas cost.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:11-15
Timestamp: 2024-10-05T14:43:14.816Z
Learning: In the `EthereumSingleRequestProxy` contract, the state variables `payee`, `paymentReference`, `feeAddress`, `feeAmount`, and `ethereumFeeProxy` should remain public, as they need to be accessed externally.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:22-27
Timestamp: 2024-10-15T07:50:58.094Z
Learning: In the `SingleRequestProxyFactory` contract, the `paymentReference` parameter in events should remain `indexed`, even if removing `indexed` could reduce gas costs.
packages/smart-contracts/test/contracts/SingleRequestProxyFactory.test.ts (1)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/test/contracts/EthereumSingleRequestProxy.test.ts:0-0
Timestamp: 2024-10-17T18:33:41.124Z
Learning: In the `EthereumSingleRequestProxy` contract, the custom reentrancy guard is nearly identical to OpenZeppelin's ReentrancyGuard, except that it allows the first reentrancy call from the `ERC20FeeProxy`. We assume it works correctly without needing further tests.
🔇 Additional comments (5)
packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol (3)
24-27: LGTM! Event declarations enhanced with fee-related parameters.
The addition of feeAddress, feeAmount, and feeProxyUsed parameters to both events improves transparency by including all relevant fee data. The paymentReference correctly remains indexed for efficient event filtering.
Also applies to: 34-37
73-80: LGTM! Event emission includes all required parameters.
The event emission correctly includes all fee-related data and maintains proper parameter ordering.
24-37: Verify test coverage for new event parameters.
The AI summary mentions test updates, but let's verify the test coverage for the new event parameters.
✅ Verification successful
The previous search failed because we need to locate the test files first and ensure we're using the correct test assertion patterns for the framework being used. Let's cast a wider net to find relevant test coverage.
Let's specifically look for the test coverage of the SingleRequestProxy events to verify how the new event parameters are tested.
Based on the test coverage shown in the output, I can now provide a final response:
Test coverage verified for new event parameters
The test files show complete coverage for both events' parameters:
EthereumSingleRequestProxyCreatedevent is tested with all parameters includingfeeAddress,feeAmount, andfeeProxyUsedinSingleRequestProxyFactory.test.tsERC20SingleRequestProxyCreatedevent is similarly tested with all parameters including the new fee-related fields
The tests verify both event emission and parameter correctness through expect().to.emit().withArgs() assertions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for event parameter assertions in tests
rg -A 5 "expect.*event.*SingleRequestProxy(Created|Event)" test/
Length of output: 112
Script:
#!/bin/bash
# First, let's find the test files in the repository
fd -e sol -e js -e ts "test|spec"
# Then search for event assertions in test files
rg -A 5 "emit.*SingleRequestProxy"
# Also search for event parameter testing patterns
rg -A 5 "expectEmit|expect.*event"
Length of output: 119984
Script:
#!/bin/bash
# Search for SingleRequestProxy test files and their event testing
fd -e ts "SingleRequestProxy.*test"
# Search for event testing in SingleRequestProxy tests
rg -A 10 "emit.*SingleRequestProxy|expect.*SingleRequestProxy" packages/smart-contracts/test/
Length of output: 19398
packages/smart-contracts/test/contracts/SingleRequestProxyFactory.test.ts (2)
88-95: LGTM! Event assertion properly validates fee parameters.
The updated event assertion correctly validates all parameters including the new fee-related fields (feeRecipientAddress, feeAmount, ethereumFeeProxy.address). This aligns with the contract changes to include fee information in deployment events.
130-138: LGTM! Event assertion properly validates ERC20 proxy parameters.
The updated event assertion correctly validates all parameters including token address and fee-related fields. The test provides good coverage by checking both the event emission and the resulting proxy state.
MantisClone
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
packages/payment-processor/test/payment/single-request-proxy.test.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/payment-processor/test/payment/single-request-proxy.test.ts (4)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:251-270
Timestamp: 2024-10-28T16:03:33.215Z
Learning: When testing the payment-processor module, specifically in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to omit tests for partial payments if they have already been covered at the smart-contract level.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:156-160
Timestamp: 2024-10-28T15:52:05.032Z
Learning: When determining if a `SingleRequestProxy` is an `ERC20SingleRequestProxy` or an `EthereumSingleRequestProxy`, include a comment explaining that the presence of `tokenAddress` distinguishes between the two.
Learnt from: aimensahnoun
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:202-209
Timestamp: 2024-10-29T09:00:54.169Z
Learning: In the `packages/payment-processor/src/payment/single-request-proxy.ts` file, within the `payWithEthereumSingleRequestProxy` function, the current error handling is acceptable as per the project's expectations.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts:150-152
Timestamp: 2024-10-17T18:30:55.410Z
Learning: In `packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts`, the skipped test `'should process a partial payment correctly'` exists intentionally to show that partial payments are supported without duplicating previous happy-path tests.
packages/payment-processor/test/payment/single-request-proxy.test.ts
Outdated
Show resolved
Hide resolved
packages/payment-processor/test/payment/single-request-proxy.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
packages/payment-processor/test/payment/single-request-proxy.test.ts(2 hunks)packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol(1 hunks)packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol(1 hunks)packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol(3 hunks)
🧰 Additional context used
📓 Learnings (4)
packages/payment-processor/test/payment/single-request-proxy.test.ts (5)
Learnt from: aimensahnoun
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/test/contracts/EthereumSingleRequestProxy.test.ts:0-0
Timestamp: 2024-10-17T16:01:47.326Z
Learning: The `rescueFunds` function in `EthereumSingleRequestProxy` does not emit any events.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:251-270
Timestamp: 2024-10-28T16:03:33.215Z
Learning: When testing the payment-processor module, specifically in `packages/payment-processor/test/payment/single-request-proxy.test.ts`, it's acceptable to omit tests for partial payments if they have already been covered at the smart-contract level.
Learnt from: aimensahnoun
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:202-209
Timestamp: 2024-10-29T09:00:54.169Z
Learning: In the `packages/payment-processor/src/payment/single-request-proxy.ts` file, within the `payWithEthereumSingleRequestProxy` function, the current error handling is acceptable as per the project's expectations.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:156-160
Timestamp: 2024-10-28T15:52:05.032Z
Learning: When determining if a `SingleRequestProxy` is an `ERC20SingleRequestProxy` or an `EthereumSingleRequestProxy`, include a comment explaining that the presence of `tokenAddress` distinguishes between the two.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts:150-152
Timestamp: 2024-10-17T18:30:55.410Z
Learning: In `packages/smart-contracts/test/contracts/ERC20SingleRequestProxy.test.ts`, the skipped test `'should process a partial payment correctly'` exists intentionally to show that partial payments are supported without duplicating previous happy-path tests.
packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol (2)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:11-15
Timestamp: 2024-10-05T14:43:14.816Z
Learning: In the `EthereumSingleRequestProxy` contract, the state variables `payee`, `paymentReference`, `feeAddress`, `feeAmount`, and `ethereumFeeProxy` should remain public, as they need to be accessed externally.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:22-27
Timestamp: 2024-10-15T07:50:58.094Z
Learning: In the `SingleRequestProxyFactory` contract, the `paymentReference` parameter in events should remain `indexed`, even if removing `indexed` could reduce gas costs.
packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol (5)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:11-15
Timestamp: 2024-10-05T14:43:14.816Z
Learning: In the `EthereumSingleRequestProxy` contract, the state variables `payee`, `paymentReference`, `feeAddress`, `feeAmount`, and `ethereumFeeProxy` should remain public, as they need to be accessed externally.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:19-23
Timestamp: 2024-10-05T14:43:07.965Z
Learning: In the `EthereumSingleRequestProxy` contract, a custom reentrancy guard is used instead of OpenZeppelin's `ReentrancyGuard`, with minor edits to allow reentrancy by the `ethereumFeeProxy`.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:77-78
Timestamp: 2024-10-15T07:50:54.734Z
Learning: In the `receive()` function of the `EthereumSingleRequestProxy` contract (`packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol`), the current error handling for the call to `ethereumFeeProxy` is considered sufficient, and no additional error handling is required.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:0-0
Timestamp: 2024-09-27T11:42:01.062Z
Learning: In the `createEthereumSingleRequestProxy` function, it's acceptable for `_feeAddress` to be the zero address, as setting the fee address to zero is a common practice. Adding zero address checks is unnecessary and adds extra gas cost.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/test/contracts/EthereumSingleRequestProxy.test.ts:0-0
Timestamp: 2024-10-17T18:33:41.124Z
Learning: In the `EthereumSingleRequestProxy` contract, the custom reentrancy guard is nearly identical to OpenZeppelin's ReentrancyGuard, except that it allows the first reentrancy call from the `ERC20FeeProxy`. We assume it works correctly without needing further tests.
packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol (3)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:0-0
Timestamp: 2024-09-27T11:42:01.062Z
Learning: In the `createEthereumSingleRequestProxy` function, it's acceptable for `_feeAddress` to be the zero address, as setting the fee address to zero is a common practice. Adding zero address checks is unnecessary and adds extra gas cost.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol:11-15
Timestamp: 2024-10-05T14:43:14.816Z
Learning: In the `EthereumSingleRequestProxy` contract, the state variables `payee`, `paymentReference`, `feeAddress`, `feeAmount`, and `ethereumFeeProxy` should remain public, as they need to be accessed externally.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1453
File: packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol:22-27
Timestamp: 2024-10-15T07:50:58.094Z
Learning: In the `SingleRequestProxyFactory` contract, the `paymentReference` parameter in events should remain `indexed`, even if removing `indexed` could reduce gas costs.
🔇 Additional comments (8)
packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol (2)
16-16: LGTM! Public visibility maintained for external access.
The addition of bytes public paymentReference follows the established pattern of public state variables, which is correct as per previous implementations.
16-16: Verify event emission in factory contract.
These changes support the PR objective of adding data to deployment events, but we should verify that the factory contract properly emits events with this data.
Also applies to: 24-24, 31-31
✅ Verification successful
Event emission is properly implemented
The factory contract (SingleRequestProxyFactory.sol) correctly defines and emits the ERC20SingleRequestProxyCreated event with the paymentReference parameter. The event is emitted in the createERC20SingleRequestProxy function with all required parameters, including the payment reference data from the proxy contract.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the factory contract properly emits events with the new payment reference
ast-grep --pattern 'event ERC20SingleRequestProxyCreated(
$$$
bytes indexed paymentReference,
$$$
)'
Length of output: 108
Script:
#!/bin/bash
# Let's try a broader search for the event definition and its emission
rg -A 5 "event ERC20SingleRequestProxyCreated"
# Also search for any emit statements related to this event
rg -A 5 "emit ERC20SingleRequestProxyCreated"
# Let's also check the factory contract implementation
fd -e sol "Factory" --exec cat {}
Length of output: 5605
packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol (4)
24-27: LGTM: Event declarations properly enhanced with fee parameters.
The addition of feeAddress, feeAmount, and feeProxyUsed parameters to both events aligns with the PR objectives and will enable proper indexing by the payment-subgraph. The indexed keyword is correctly maintained on paymentReference as required.
Also applies to: 34-37
70-79: LGTM: Proxy creation and event emission properly updated.
The changes correctly:
- Update constructor parameters to match the proxy's signature
- Emit the event with all required fee-related data
109-117: LGTM: Event emission properly includes fee parameters.
The ERC20SingleRequestProxyCreated event emission correctly includes all necessary parameters, maintaining consistency with the Ethereum proxy implementation.
Line range hint 24-117: Verify completeness of event emission updates.
Let's ensure no other event emissions were missed in the codebase.
✅ Verification successful
Event emissions are complete and correctly implemented
After reviewing the code, I can confirm that:
- The
SingleRequestProxyFactorycontract correctly emits bothEthereumSingleRequestProxyCreatedandERC20SingleRequestProxyCreatedevents with all required parameters when creating respective proxies. - The proxy contracts themselves (
EthereumSingleRequestProxyandERC20SingleRequestProxy) don't need to emit these events as they are correctly emitted by the factory. - All event parameters match the contract state and function parameters.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other event emissions that might need similar updates
rg -A 2 "emit (Ethereum|ERC20)SingleRequestProxyCreated" --type sol
Length of output: 97
Script:
#!/bin/bash
# Search for event emissions in Solidity files using a more generic pattern
rg -t solidity -A 2 "emit (Ethereum|ERC20)SingleRequestProxyCreated"
Length of output: 103
Script:
#!/bin/bash
# List all Solidity files to ensure we're searching in the right place
fd -e sol
Length of output: 2739
Script:
#!/bin/bash
# Alternative search using ast-grep for event emissions
ast-grep --pattern 'emit $_ ($$$)'
Length of output: 36
Script:
#!/bin/bash
# Search for event emissions in the SingleRequestProxyFactory.sol file
cat packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol
Length of output: 4406
Script:
#!/bin/bash
# Also check the proxy contracts for any related event emissions
cat packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol
cat packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol
Length of output: 6639
packages/payment-processor/test/payment/single-request-proxy.test.ts (2)
192-195: LGTM! Event data decoding is correctly implemented.
The decoder parameters correctly match the expected event structure for EthereumSingleRequestProxyCreated, including the fee-related fields.
231-234: LGTM! Event data decoding is correctly implemented.
The decoder parameters correctly match the expected event structure for ERC20SingleRequestProxyCreated, including the fee-related fields.
packages/payment-processor/test/payment/single-request-proxy.test.ts
Outdated
Show resolved
Hide resolved
packages/payment-processor/test/payment/single-request-proxy.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
packages/payment-processor/src/payment/single-request-proxy.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/payment-processor/src/payment/single-request-proxy.ts (5)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:156-160
Timestamp: 2024-10-28T15:52:05.032Z
Learning: When determining if a `SingleRequestProxy` is an `ERC20SingleRequestProxy` or an `EthereumSingleRequestProxy`, include a comment explaining that the presence of `tokenAddress` distinguishes between the two.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:237-246
Timestamp: 2024-11-08T18:24:06.144Z
Learning: In `packages/payment-processor/test/payment/single-request-proxy.test.ts`, when asserting the `feeProxyUsed` in emitted events from `SingleRequestProxyFactory`, retrieve the `erc20FeeProxy` (or `ethereumFeeProxy`) public variable from the `SingleRequestProxyFactory` contract instead of using `wallet.address`.
Learnt from: aimensahnoun
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:202-209
Timestamp: 2024-10-29T09:00:54.169Z
Learning: In the `packages/payment-processor/src/payment/single-request-proxy.ts` file, within the `payWithEthereumSingleRequestProxy` function, the current error handling is acceptable as per the project's expectations.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/payment-processor/src/payment/single-request-proxy.ts:156-160
Timestamp: 2024-10-28T15:52:05.032Z
Learning: In the `payRequestWithSingleRequestProxy` function of `single-request-proxy.ts`, failing to retrieve `tokenAddress()` is expected for `EthereumSingleRequestProxy` contracts and is not an error.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1487
File: packages/payment-processor/test/payment/single-request-proxy.test.ts:198-206
Timestamp: 2024-11-08T18:24:19.095Z
Learning: In tests for `SingleRequestProxyFactory`, the `feeProxyUsed` in events should be verified by retrieving the `ethereumFeeProxy` public variable from the `SingleRequestProxyFactory` contract, not `wallet.address`.
aimensahnoun
force-pushed
the
update-srp-to-index-all-public-variables
branch
from
b61d1ed
to
93279f2
Compare
Towards #1283
Problem
The payment-subgraph could not index the feeAmount, feeAddress, and feeProxyUsed because they were not included in the deployment event.
Description of the changes
Summary by CodeRabbit
Summary by CodeRabbit
New Features
paymentReferencein theERC20SingleRequestProxycontract for better payment tracking.Bug Fixes
Chores
sepolianetwork in the artifact configuration.