Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

hardhat/tasks/misc.js Outdated Show resolved Hide resolved
@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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.