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

SOV-4378 utils hh tasks #550

Open
wants to merge 7 commits into
base: bobDevelopment
Choose a base branch
from
Open

Conversation

jjmr007
Copy link
Contributor

@jjmr007 jjmr007 commented Dec 11, 2024

As required in the Description of the SOV-4378 Jira ticket, a pull request is shared for review.

In this PR the hh tasks utils:send-direct, canceltx and droptx were included in utils.js, among other changes in hardhat config, and package.json to enable an easier access to the commands.

Copy link
Contributor

@cwsnt cwsnt left a comment

Choose a reason for hiding this comment

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

  • Consider change the console.log to use the logger / node-logs and apply the severity respectively (e.g: logger.warn() / logger.error() / logger.info())
  • Remove the unnecessary comment

"0x5684a06CaB22Db16d901fEe2A5C081b4C91eA40e"
);
//const abi = (await deployments.getArtifact("Staking")).abi;
const abi = ["event VestingStakeSet(uint256,uint96)"];
Copy link
Contributor

Choose a reason for hiding this comment

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

can we get the ABI from the deployment file instead? to be more reliable in case the event is changing

Copy link
Contributor Author

@jjmr007 jjmr007 Dec 17, 2024

Choose a reason for hiding this comment

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

Not sure about who the original author of this hh task was (was me? crap!)
But seems that this abi element is never used in the current task. The only thing I did in this commit was to improve the name of the task to fit the format. Also seems the task needs additional refactor, coz is not working.
P.S.: Seems the key is in the "cblock" parameter. It may require a function to automatically detect what the right block is, but I'm affraid it is beyond the acceptance criteria of this Jira task

} = hre;
const staking = await ethers.getContractAt(
"IStaking",
"0x5684a06CaB22Db16d901fEe2A5C081b4C91eA40e"
Copy link
Contributor

Choose a reason for hiding this comment

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

Read the address from the deployment file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved

cblock += 10000;
if (cblock > block) cblock = block;
const filter = {
address: "0x5684a06CaB22Db16d901fEe2A5C081b4C91eA40e",
Copy link
Contributor

Choose a reason for hiding this comment

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

Read from the deployment file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved

//console.log(await getEthersLog(staking, filter));
if (cres[0]) {
console.log("index: ", index++, "\n", cres);
//break;
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove unnecessary comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved

.setAction(async (taskArgs, hre) => {
const { NODE_REAL_API_KEY } = process.env;
const forkParams = {
jsonRpcUrl: `https://eth-mainnet.nodereal.io/v1/${NODE_REAL_API_KEY}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

probably it's a good idea to put all of the fork rpc base url to a config. So that it can be reused, and easier to update.
Something like

const forkRpcUrls = {
"ethMainnet": "https:xxx",
"bobMainnet": "https:xxx",
....
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved

@jjmr007
Copy link
Contributor Author

jjmr007 commented Dec 17, 2024

  • Consider change the console.log to use the logger / node-logs and apply the severity respectively (e.g: logger.warn() / logger.error() / logger.info())
  • Remove the unnecessary comment

@cwsnt : Please check last commit: 1899f9cf201b102fb4491cc4a8a0b2ad59e52569 with all the required changes done.

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