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

fix(builder): validation of template accesses which depend on chainId #1691

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

Conversation

dbeal-eth
Copy link
Contributor

or other globals

this basically turned into a refactor because we have to kind of rethink how we are supplying values to the ChainDefinition. But I think its a move in the right direction.

Unfortunately refactors like this usually lead to breakage, so probably best to save this for next release.

or other globals

this basically turned into a refactor because we have to kind of rethink
how we are supplying values to the ChainDefinition. But I think its a
move in the right direction.

Unfortunately refactors like this usually lead to breakage, so probably
best to save this for next release.
@dbeal-eth dbeal-eth added the bug Something isn't working label Feb 12, 2025
@dbeal-eth dbeal-eth self-assigned this Feb 12, 2025
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@radix-ui/[email protected] None +11 243 kB chancestrickland
npm/@radix-ui/[email protected] None +1 32.3 kB vladmoroz
npm/@radix-ui/[email protected] None +10 183 kB chancestrickland
npm/@radix-ui/[email protected] None +9 173 kB chancestrickland
npm/@radix-ui/[email protected] None +14 384 kB chancestrickland
npm/@radix-ui/[email protected] None +28 1.37 MB chancestrickland
npm/@radix-ui/[email protected] environment 0 3.44 MB chancestrickland
npm/@radix-ui/[email protected] None +1 30.3 kB vladmoroz
npm/@radix-ui/[email protected] None +24 1.01 MB chancestrickland
npm/@radix-ui/[email protected] None +9 318 kB chancestrickland
npm/@radix-ui/[email protected] None +28 1.26 MB chancestrickland
npm/@radix-ui/[email protected] None +1 34 kB vladmoroz
npm/@radix-ui/[email protected] None +1 33.2 kB vladmoroz
npm/@radix-ui/[email protected] None +10 166 kB chancestrickland
npm/@radix-ui/[email protected] None +13 306 kB chancestrickland
npm/@radix-ui/[email protected] None +14 451 kB chancestrickland
npm/@radix-ui/[email protected] None +23 993 kB chancestrickland
npm/@rainbow-me/[email protected] environment, network 0 3.16 MB danielsinclair
npm/@rollup/[email protected] None +1 67.8 kB shellscape
npm/@rollup/[email protected] filesystem +1 165 kB shellscape
npm/@rollup/[email protected] eval, unsafe 0 28.7 kB lukastaegert
npm/@safe-global/[email protected] None +4 3.95 MB dasanra
npm/@safe-global/[email protected] None +10 3.5 MB dasanra
npm/@safe-global/[email protected] Transitive: network +1 385 kB dasanra
npm/@sentry/[email protected] environment, filesystem, network Transitive: unsafe +9 13.9 MB sentry-bot

🚮 Removed packages: npm/@types/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

@@ -15,7 +15,7 @@ options.salt = "second"
options.msg = "a message from second greeter set"

[invoke.do_change_greeting2]
target = ["greeters.Greeter"]
target = ["greeters.Greeterz"]
Copy link
Member

Choose a reason for hiding this comment

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

Is this ok?

Comment on lines +111 to +115
new ChainDefinition(deployInfo.def, false, {
chainId,
timestamp: deployInfo.timestamp || 0,
package: { version: '0.0.0' },
}),
Copy link
Member

Choose a reason for hiding this comment

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

Having to create this dummy object on several places just to be able to instantiate looks prone to error, maybe a helper function to create the ChainDefinition would be useful?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see that those are overrides, but are they necessary? looks like they are only used for giving them to the access recorder, in that case wouldn't be enough to just send the dummy object? Why is there an option to send custom overrides?

@@ -1,4 +1,4 @@
name = "greeter-foundry"
name = "greeter"
version = "<%= package.version %>"
description = "Simple project to verify the functionality of cannon"
keywords = ["sample", "greeter"]
Copy link
Member

@mjlescano mjlescano Feb 12, 2025

Choose a reason for hiding this comment

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

Would be good to add an action on this or other of the cannonfiles used on the test with Inifinex's case, was is with something like <%= settings.smth['VALUE_' + chainId] %>?

@mjlescano
Copy link
Member

Something appears to be fishy? socket reports a lot of dependencies changes here: #1691 (comment), but there are no changes on any package.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants