Skip to content

Conversation

@legobeat
Copy link
Contributor

@legobeat legobeat commented May 28, 2024

  • Remove web3 property
  • Make all private properties publically inaccessible
  • Mark read-only private functions as readonly
  • Add setProvider function which allows atomically setting the provider and block tracker on an existing instance.

Should remove the need for this patch in MM extension.


Blocking

@legobeat legobeat force-pushed the dynamic-provider-private-props branch from bc4ecce to 3de3044 Compare May 28, 2024 07:34
legobeat added 2 commits May 28, 2024 16:36
If anyone would ever update the provider, they must also update the
blockTracker
@legobeat legobeat force-pushed the dynamic-provider-private-props branch from 3de3044 to 734129a Compare May 28, 2024 07:36
@legobeat legobeat marked this pull request as ready for review May 28, 2024 07:36
@legobeat legobeat requested a review from a team as a code owner May 28, 2024 07:36
@legobeat legobeat requested review from a team, OGPoyraz, kumavis, matthewwalsh0 and vinistevam May 28, 2024 07:36
@legobeat legobeat requested review from Mrtenz and mcmire May 28, 2024 07:55
Copy link
Member

@Mrtenz Mrtenz left a comment

Choose a reason for hiding this comment

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

This is technically a breaking change, right? Looks good to me though.

* @param opts.provider - An ethereum provider
* @param opts.blockTracker - An instance of @metamask/eth-block-tracker
*/
setProvider({
Copy link
Contributor Author

@legobeat legobeat May 28, 2024

Choose a reason for hiding this comment

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

I am very easily convinced to remove the setProvider function alltogether, if there is no current use for it. Put it in to make sure there are no compatibility concerns by default.

@legobeat
Copy link
Contributor Author

@Mrtenz Indeed, and intended to be released as such. setProvider is the intended migration-hatch if it turns out to be actually-breaking anywhere.

@legobeat legobeat merged commit 729ddd9 into MetaMask:main May 28, 2024
Comment on lines -211 to +230
const blockNumber = await this.blockTracker.getLatestBlock();
const baseCount: number = await this.web3.getTransactionCount(
address,
blockNumber,
);
const blockNumber = await this.#blockTracker.getLatestBlock();
const baseCount: number = await new Web3Provider(
this.#provider,
).getTransactionCount(address, blockNumber);

This comment was marked as off-topic.

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.

3 participants