8000 Return results in Operation event for visibility of actions by jacque006 · Pull Request #336 · getwax/bls-wallet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Nov 5, 2023. It is now read-only.

Return results in Operation event for visibility of actions #336

Merged
merged 5 commits into from
Sep 16, 2022
Merged

Return results in Operation event for visibility of actions #336

merged 5 commits into from
Sep 16, 2022

Conversation

jacque006
Copy link
Collaborator
@jacque006 jacque006 commented Sep 14, 2022 •
edited by jzaki
Loading

What is this PR doing?

Making actions and (failing results) more accessible.

How can these changes be manually tested?

Unit test for failing action.
Look at Operation event logs of for action data when all succeed, and an error message when one action fails.

Does this PR resolve or contribute to any issues?

Closes #193

Checklist

  • I have manually tested these changes
  • Post a link to the PR in the group chat

Guidelines

  • If your PR is not ready, mark it as a draft
  • The resolve conversation button is for reviewers, not authors
    • (But add a 'done' comment or similar)

@jzaki
Return results in Operation event for visibility of failing actions
< 8000 input type="hidden" name="disable_live_updates" value="false" autocomplete="off" data-targets="batch-deferred-content.inputs" />
ce8620c
@github-actions github-actions bot added the contracts Smart contract related label Sep 14, 2022
@jzaki jzaki marked this pull request as ready for review September 14, 2022 13:43
@jzaki jzaki changed the title Return results in Operation event for visibility of failing actions Return results in Operation event for visibility of actions Sep 14, 2022
blakecduncan
blakecduncan approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor
@blakecduncan blakecduncan left a comment

Choose a reason for hiding this comment

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

I'm not the best with solidity but logically this makes sense 👍

@jzaki
Copy link
Collaborator
jzaki commented Sep 15, 2022

Who can update what is required on the client side for this PR to be merged? @jacque006 @kautukkundan @voltrevo

jacque006
jacque006 commented Sep 16, 2022
View reviewed changes
contracts/test/walletAction-test.ts
["string"],
errorArgBytesString,
)[0]; // decoded bytes is a string of the action index that errored.
expect(errorString).to.equal("1 - ERC20: transfer from the zero address");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🎉

@jacque006
Copy link
Collaborator Author

@kautukkundan Will update

@github-actions github-actions bot added aggregator Aggregator backend related aggregator-proxy Aggregator proxy related clients extension Browser extension related labels Sep 16, 2022
@jacque006 jacque006 merged commit dcfd13d into main Sep 16, 2022
@jacque006 jacque006 deleted the contract-operation-logs branch September 16, 2022 17:37
@kautukkundan kautukkundan restored the contract-operation-logs branch September 22, 2022 11:24
@jzaki jzaki deleted the contract-operation-logs branch December 2, 2022 10:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blakecduncan blakecduncan blakecduncan approved these changes

@voltrevo voltrevo Awaiting requested review from voltrevo

Assignees
No one assigned
Labels
aggregator Aggregator backend related aggregator-proxy Aggregator proxy related clients contracts Smart contract related extension Browser extension related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Emit event for each bundle action performed by BLSWallet
4 participants
@jacque006 @jzaki @blakecduncan @kautukkundan
0