-
Notifications
You must be signed in to change notification settings - Fork 24
percentile 80, bufferPercent multiply #267
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
percentile 80, bufferPercent multiply #267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 7
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: Incomplete signature update breaks fallback gas price paths
The applyGasPriceBuffer function signature was changed from 2 parameters to 3 parameters (maxBaseFee, maxPriorityFee, gasPriceBufferPercent), but two fallback call sites weren't updated. These calls still pass only 2 arguments, causing gasPriceBufferPercent to become the maxPriorityFee parameter (a number), and the actual third parameter becomes undefined. When the function executes maxBaseFee + maxPriorityFee, it attempts to add a BigInt with a number, which throws a TypeError.
services/blockchain-service/blockchain-service-base.js#L1461-L1465
dkg.js/services/blockchain-service/blockchain-service-base.js
Lines 1461 to 1465 in 7435a82
| if (baseFees.length === 0 || priorityFees.length === 0) { | |
| return this.applyGasPriceBuffer( | |
| BigInt(await this.getNetworkGasPrice(blockchain)), | |
| gasPriceBufferPercent, | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Mismatched function call arguments cause runtime error
The applyGasPriceBuffer function signature was changed from 2 to 3 parameters, but two fallback call sites were not updated. These calls still pass (gasPrice, gasPriceBufferPercent) instead of (maxBaseFee, maxPriorityFee, gasPriceBufferPercent). This causes maxPriorityFee to receive the buffer percent number while gasPriceBufferPercent becomes undefined. When the function attempts to add maxBaseFee (BigInt) to maxPriorityFee (now a number), JavaScript will throw a TypeError because BigInt and Number types cannot be mixed.
services/blockchain-service/blockchain-service-base.js#L1462-L1466
dkg.js/services/blockchain-service/blockchain-service-base.js
Lines 1462 to 1466 in a5ad0a4
| if (baseFees.length === 0 || priorityFees.length === 0) { | |
| return this.applyGasPriceBuffer( | |
| BigInt(await this.getNetworkGasPrice(blockchain)), | |
| gasPriceBufferPercent, | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Fallback calls use wrong argument count for updated function
The applyGasPriceBuffer function signature changed from 2 to 3 parameters (maxBaseFee, maxPriorityFee, gasPriceBufferPercent), but the fallback call sites at lines 1453-1456 and 1463-1466 still pass only 2 arguments. This causes gasPriceBufferPercent to be passed as maxPriorityFee, leaving the third argument undefined. When the function executes maxBaseFee + maxPriorityFee (since gasPriceBufferPercent is falsy), it will try to add a BigInt with a Number, throwing a TypeError.
services/blockchain-service/blockchain-service-base.js#L1452-L1456
dkg.js/services/blockchain-service/blockchain-service-base.js
Lines 1452 to 1456 in 827f326
| if (!feeHistory.supported) { | |
| return this.applyGasPriceBuffer( | |
| BigInt(await this.getNetworkGasPrice(blockchain)), | |
| gasPriceBufferPercent, | |
| ); |
services/blockchain-service/blockchain-service-base.js#L1462-L1466
dkg.js/services/blockchain-service/blockchain-service-base.js
Lines 1462 to 1466 in 827f326
| if (baseFees.length === 0 || priorityFees.length === 0) { | |
| return this.applyGasPriceBuffer( | |
| BigInt(await this.getNetworkGasPrice(blockchain)), | |
| gasPriceBufferPercent, | |
| ); |
| 0n, | ||
| BigInt(await this.getNetworkGasPrice(blockchain)), | ||
| gasPriceBufferPercent, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Buffer not applied to fallback network gas price
In the fallback cases, the network gas price is passed as maxPriorityFee (second argument) while maxBaseFee is 0n. Since applyGasPriceBuffer only applies the buffer percentage to maxBaseFee, the calculation becomes ((0n * buffer) / 100n) + networkGasPrice = networkGasPrice. The buffer is never applied to the fallback gas price. The arguments appear to be in the wrong order—the network gas price should be passed as maxBaseFee and 0n as maxPriorityFee.
Note
Use a configurable priority fee percentile (default 80) for fee history and apply gas buffer to base fee only, with config plumbed via InputService.
services/blockchain-service/blockchain-service-base.js):blockchain.priorityFeePercentile(default80) ineth_getFeeHistoryinstead of fixed50.applyGasPriceBuffer(maxBaseFee, maxPriorityFee, gasPriceBufferPercent)to apply buffer only to base fee and then add priority fee.estimateGasPriceFromFeeHistoryto use new signature and handle fallbacks by separating base and priority components.services/input-service.js):priorityFeePercentileingetBlockchainconfig.Written by Cursor Bugbot for commit 0e6a648. This will update automatically on new commits. Configure here.