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

Some default properties cannot be set from Contract class #5603

Closed
1 task done
nikoulai opened this issue Nov 10, 2022 · 3 comments
Closed
1 task done

Some default properties cannot be set from Contract class #5603

nikoulai opened this issue Nov 10, 2022 · 3 comments
Assignees
Labels
4.x 4.0 related Bug Addressing a bug

Comments

@nikoulai
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

During writing some tests for contract defaults, noticed that defaultCommon and defaultHardfork didn't set properly from the Contract class and updated to the instance of the class.

Expected Behavior

The values setted in the Contract class should be present in the instance, too.

Steps to Reproduce

Run the two commented tests in packages/web3-eth-contract/test/integration/contract_defaults_extra.test.ts

Web3.js Version

4.0.1-alpha.1

Environment

  • Operating System: macOs
  • Browser:
  • Node.js Version: v16.14.2
  • NPM Version: 8.5.0

Anything Else?

No response

@Muhammad-Altabba
Copy link
Contributor

This could be because of the same root problem of #5433

@Muhammad-Altabba
Copy link
Contributor

Hello all,
I was trying to do simple modifications to make things works for the defaultCommon. But it seems to need more refactoring. So, I am closing my MR (#5688) and I think we need to do it some other way. (Additionally, I am unassigning this issue from me since I will be on vacation. But, I think the one who will take it would treat it as a fresh task but with some insights)

I think the configurations are better to be totally refactored to have:

The above are just some thoughts. The one who works on this would probably need to do it this was or totally another 😄.

@Muhammad-Altabba Muhammad-Altabba removed their assignment Dec 9, 2022
@spacesailor24
Copy link
Contributor

Maybe this comment could also be addressed during this refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x 4.0 related Bug Addressing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants