feat: tron batch payments contracts and tests#1724
Conversation
Greptile SummaryThis PR introduces
Confidence Score: 5/5The batch contract logic is sound and the deployed addresses in the artifact registry are now consistent; the only open item is a testnet-script convenience concern. The contract's token-aggregation and payment-dispatch paths are correct, the custom SafeERC20 wrapper handles non-standard tokens safely, and all previously flagged issues (artifact name mismatch, threw undefined, nile address mismatch) are resolved in this revision. The single remaining note — the nile deployment script unconditionally redeploys ERC20FeeProxy — is a developer-tooling concern that does not affect the on-chain contract or the mainnet deployment path. packages/smart-contracts/scripts/tron/deploy-nile.js — lacks the selective-deploy guard that the mainnet script uses, so a re-run would overwrite nile.json with a fresh proxy address. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Caller invokes batch function] --> B{Validate array lengths}
B -->|Mismatch| C[Revert]
B -->|OK| D[Compute total per token]
D --> E{For each unique token}
E --> F{Check allowance}
F -->|Insufficient| C
F -->|OK| G{Check balance}
G -->|Insufficient| C
G -->|OK| H[Pull tokens into batch contract]
H --> I{Proxy allowance enough?}
I -->|No| J[Approve proxy for max]
I -->|Yes| K[Dispatch payments via ERC20FeeProxy]
J --> K
K --> L[Done]
Reviews (7): Last reviewed commit: "update nile ERC20BatchPAyments deploymen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const expectRevertOrNoBalanceChange = async (fn, getBalances) => { | ||
| const before = await getBalances(); | ||
| try { | ||
| await fn(); | ||
| } catch (_error) {} | ||
| await waitForConfirmation(2000); | ||
| const after = await getBalances(); | ||
| const unchanged = before.every((value, index) => value === after[index]); | ||
| return { unchanged }; | ||
| }; |
There was a problem hiding this comment.
expectRevertOrNoBalanceChange catches errors without asserting that a revert actually occurred. If the transaction unexpectedly succeeds and the balance check happens to track the wrong account, a funds leak would be silently accepted. The expectNonOwnerReverts helper in the same file correctly asserts threw === true; applying the same pattern here would make failure detection explicit.
There was a problem hiding this comment.
On Tron, when a revert occurs the transaction call does not fail.
See this transaction for example.
|
@LeoSlrRf is this still true? If not, please update the PR description. > Currently, both contracts provide methods for managing the underlying proxies. |
|
@MantisClone It wasn't the case. I just pushed the ownership removal and updated the PR description |
| uint256[] calldata _amounts, | ||
| bytes[] calldata _paymentReferences, | ||
| uint256[] calldata _feeAmounts, | ||
| address _feeAddress |
There was a problem hiding this comment.
I noticed that all batch payment contracts enforce a single feeAddress for all payments. I don't see why that would be the case.
Given that we won't use the fee parameters, I'm wondering if it's worth updating to an array for Tron only.
|
Post-approval optional hardening note:
|
2 participants
Description of the changes
Adds Tron-specific batch payment contracts and test infrastructure to the
smart-contractspackage.This PR introduces
ERC20BatchPayments,an ERC20-specific version of the BatchPayments contract.This version offers better performance and increased security on Tron.
ERC20BatchPayments.test.jscoversERC20BatchPayments, including happy-path single-token and multi-token payments, zero-amount payments, zero-fee payments,BadTRC20token handling, and error cases such as insufficient funds, missing allowance, and mismatched input arrays.A shared
helpers.jsmodule provides utilities, including token deployment, approval helpers, balance diffing, batch fee computation, and revert/no-balance-change assertion helpers.The root
package.jsonworkspace configuration is updated to prevent hoisting of@openzeppelindependencies for thesmart-contractspackage, ensuring the Tron build resolves its own copy of those contracts.The Trondeployments scripts have been updated to support the deployment of
ERC20BatchPaymentsContracts Scope
This is the same contract as the
BatchPaymentscontract from EVM without th following features:EthFeeProxymethodsSecurity Consideration
The
ERC20BatchPaymentcontract no longer has an owner compared to the original version, reducing the attack surface.Gas Consideration
ERC20BatchPaymenthas a lower gas footprint asBatchPaymentsas it does not include batch-fee-related logic.Deloyment Information
Nile: https://nile.tronscan.org/#/contract/TDnU5eY8Et3QdZRWMSTvoXQnxQeMxF7CE4
Tron: https://tronscan.org/#/contract/TUdcGd29QpV65MkbqgBLWJKbTG3UL7PuQB
Both contracts are verified ✅