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

cmd/clef: fix JS issues in rules.md #30980

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

savvar9991
Copy link

1. Fixed punctuation in header:

-The two main features that are required for this to work well are;
+The two main features that are required for this to work well are:

Explanation: Replaced semicolon with colon since it introduces a list of items.

2. Updated BigNumber constructor usage

-var limit = big.NewInt("0xb1a2bc2ec50000")
+var limit = new BigNumber("0xb1a2bc2ec50000")

Explanation: The code was using an incorrect method big.NewInt() which doesn't exist in the BigNumber.js library. The correct way to create a BigNumber instance is using the new BigNumber() constructor. This is particularly important when dealing with large numbers in Ethereum transactions where precision is crucial. The value "0xb1a2bc2ec50000" represents a hexadecimal number that needs to be handled as a big number to avoid JavaScript's number precision limitations.

3. Fixed condition syntax:

-if (req.transaction.to.toLowerCase() == "0xae967917c465db8578ca9024c205720b1a3651a9") && value.lt(limit)) {
+if (req.transaction.to.toLowerCase() == "0xae967917c465db8578ca9024c205720b1a3651a9" && value.lt(limit)) {

Explanation: The original code had an extra parenthesis that created incorrect grouping in the logical expression. In JavaScript, this could lead to unexpected behavior or syntax errors. The condition checks two things: first, if the lowercase transaction address matches a specific address, and second, if the value is less than the limit using BigNumber's lt() (less than) method. The fixed version properly groups these conditions with the logical AND (&&) operator.

@savvar9991 savvar9991 requested a review from holiman as a code owner January 3, 2025 09:38
@s1na
Copy link
Contributor

s1na commented Jan 3, 2025

Lol is this copilot generated description? The code change looks good to me. It's true big.NewInt is go syntax not JS.

@savvar9991
Copy link
Author

Lol is this copilot generated description? The code change looks good to me. It's true big.NewInt is go syntax not JS.

I'm sorry, I didn't notice

@savvar9991 savvar9991 closed this Jan 3, 2025
@s1na s1na reopened this Jan 3, 2025
@s1na
Copy link
Contributor

s1na commented Jan 3, 2025

Lol is this copilot generated description? The code change looks good to me. It's true big.NewInt is go syntax not JS.

I'm sorry, I didn't notice

I think there was a misunderstanding. Your code changes are correct.

@fjl fjl changed the title Fix BigNumber instantiation and logical condition syntax cmd/clef: fix JS issues in rules.md Jan 3, 2025
@Kya123iu Kya123iu mentioned this pull request Jan 3, 2025
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.

3 participants