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

refactor(common): simplify writeContract/sendTransaction actions #3043

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

frolic
Copy link
Member

@frolic frolic commented Aug 16, 2024

workaround until wevm/viem#2624 lands
fixes #3042

before:
image

after:
image

(note "module already installed" message, which came from being able to catch the decoded error)

Copy link

changeset-bot bot commented Aug 16, 2024

🦋 Changeset detected

Latest commit: 5483ba3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 25 packages
Name Type
@latticexyz/common Patch
@latticexyz/block-logs-stream Patch
@latticexyz/cli Patch
@latticexyz/config Patch
@latticexyz/dev-tools Patch
@latticexyz/faucet Patch
@latticexyz/protocol-parser Patch
@latticexyz/query Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/store Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch
mock-game-contracts Patch
ts-benchmarks Patch
@latticexyz/react Patch
@latticexyz/world-module-metadata Patch
@latticexyz/abi-ts Patch
create-mud Patch
@latticexyz/gas-report Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@frolic frolic marked this pull request as ready for review August 16, 2024 16:22
@frolic frolic requested a review from alvrs as a code owner August 16, 2024 16:22
@frolic
Copy link
Member Author

frolic commented Aug 20, 2024

this was landed in viem: wevm/viem#2624
bumping viem to pull this in here: #3038

@frolic frolic closed this Aug 20, 2024
@frolic frolic deleted the holic/write-contract-errors branch August 20, 2024 13:19
@frolic frolic restored the holic/write-contract-errors branch August 20, 2024 13:27
@frolic frolic reopened this Aug 20, 2024
@frolic
Copy link
Member Author

frolic commented Aug 20, 2024

actually, this is still useful because we call prepareTransactionRequest here, which is where estimation errors are thrown and this bypasses viem's error handling in writeContract

we won't need this once we refactor this to use the viem nonce manager and can just wrap writeContract in a queue without the additional prepare step, but will save that for a future PR

@frolic frolic changed the title fix(common): decode errors during writeContract refactor(common): simplify writeContract/sendTransaction actions Aug 20, 2024
@frolic frolic merged commit 2daaab1 into main Aug 20, 2024
12 checks passed
@frolic frolic deleted the holic/write-contract-errors branch August 20, 2024 14:28
@frolic
Copy link
Member Author

frolic commented Aug 20, 2024

ended up refactoring our approach here, since we can use our nonce manager in the same way the viem one would be used and it cleans up a lot + improves error messages

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.

Revert details for error Reason(param-type) don't make it from the System to the client
1 participant