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

Introduce tracing for front-end #1274

Merged
merged 8 commits into from
Jul 29, 2024
Merged

Introduce tracing for front-end #1274

merged 8 commits into from
Jul 29, 2024

Conversation

kronosapiens
Copy link
Member

@kronosapiens kronosapiens commented Jul 9, 2024

Add support for transaction tracing, mostly for the front-end development environment.

Companion to colonyCDapp#2691

@area
Copy link
Member

area commented Jul 17, 2024

Added initialDate config option myself because I want to try and use this today 😄

@area area force-pushed the maint/hardhat-tracer branch from 3ca22cb to d26cbc9 Compare July 18, 2024 10:02
@area
Copy link
Member

area commented Jul 18, 2024

Hmmmm, any thoughts as to what's going on with the tests in this PR?

@kronosapiens
Copy link
Member Author

@area I really don't have any idea. My first thought would be that somehow adding the chains argument to hardhat.config somehow is causing ECONNRESET, but I'm not sure what the mechanism would be there.

@kronosapiens kronosapiens force-pushed the maint/hardhat-tracer branch 9 times, most recently from 2dba0c0 to de76b6b Compare July 24, 2024 00:13
@kronosapiens kronosapiens force-pushed the maint/hardhat-tracer branch from de76b6b to d765ca1 Compare July 24, 2024 00:16
@kronosapiens
Copy link
Member Author

@area the culprit seems to be here: https://github.com/JoinColony/colonyNetwork/pull/1274/files#diff-cf4ef7c51dc9f81cad1d504da0d1c3a3437ac7b7d1374ee7127886cf1d1a5092R17

Haven't gotten to the bottom of why yet -- something about how the plugin gets initialised that affects port behavior? Perhaps you may have some insight.

@area
Copy link
Member

area commented Jul 24, 2024

So the underlying issue is some sort of timeout somewhere in the stack, of the order of 20-30 seconds. The main call that's causing it to trigger is a confirmNewHash call, which does quite a lot (deploys new contracts, a lot of chatter between contracts), and from looking at top while trying to make such a call the CPU and memory usage balloon when it's happening.

I think by including hardhat-tracer, the node ends up doing a lot of extra work that we never use, tracing the call as it's made. If I make multiple estimateGas calls for the call at the same time, several will time out after 20-30 seconds (and do so simultaneously withing milliseconds, yet between batches there's a much larger variance). I've added ternary (not an if statement, much to my despair, because of eslint) such that, if the node command is being used, we won't load hardhat-tracer, and so performance returns to where it was.

I do wonder if our remaining spottiness is as a result of this (to some extent), and we're on-the-edge, so occasionally we are hit by the timeout if we get a particularly poorly performing instance on Circle. If that's the case, I would expect using RetryProvider everywhere we can to improve reliability of the tests (and I am intending to do this).

From my testing, the node does not need to have the plugin for tracing to work, so there are no knock-on effects in terms of JoinColony/colonyCDapp#2691, which should continue to work as it. If we started to want to use programmatic access to traces in our tests, I could see that being a problem, but for now, I'm hopeful this is sufficient.

@area
Copy link
Member

area commented Jul 25, 2024

Okay, now I don't know what's going on. This branch and my relayer branch are now failing in the same way butdevelop is fine.

Am I cursed? I can't really see any common changes between the two. I have a vague solution but I'd really rather understand what's going on.

@area
Copy link
Member

area commented Jul 25, 2024

I'm fairly sure it's running out of available memory, but I'm stuck as to why. I don't believe it's a memory leak, and it doesn't seem to be due to left over data in the reputationStates SQLite DB.

@kronosapiens
Copy link
Member Author

Going back to the import style, would it be possible to do something along the lines of

task("trace", Run trace").setAction(async () => {
  require("hardhat-tracer"); // eslint-disable-line global-require

  await runSuper();
});

Not sure if it'll solve the issue but more explicitly isolating the import might help

@area
Copy link
Member

area commented Jul 25, 2024

I can't see how that would change things. Feel free to try, but I don't think it will solve it given the remaining test failure we are also seeing on an unrelated branch?

@area
Copy link
Member

area commented Jul 29, 2024

Hmmmmm. So that change means that it's not including tracer when we're doing hardhat test or hardhat coverage, and if we do we end up with (what is still a guess of) an out of memory issue.

@kronosapiens kronosapiens merged commit 11fd4a7 into develop Jul 29, 2024
2 checks passed
@kronosapiens kronosapiens deleted the maint/hardhat-tracer branch July 29, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants