Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

Commit

Permalink
Add getOperationResults to cleints
Browse files Browse the repository at this point in the history
Add getOperationResults to bls-wallet-clients to allow consumers to more easily get at operation errors.
Change existing test case to use getOperationResults.
Add message to require used to prevent ownership changes to proxy admin.
  • Loading branch information
jacque006 committed Oct 26, 2022
1 parent 4bc4701 commit 6379006
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 10 deletions.
18 changes: 18 additions & 0 deletions contracts/clients/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,24 @@ const verificationGateway = VerificationGateway__factory.connect(
await verificationGateway.processBundle(bundle);
```

You can get the results of the operations in a bundle using `getOperationResults`.

```ts
import { getOperationResults } from 'bls-wallet-clients';

...

const txn = await verificationGateway.processBundle(bundle);
const txnReceipt = txn.wait();
const opResults = getOperationResults(txnReceipt);

// Includes data from WalletOperationProcessed event,
// as well as parsed errors with action index
const { error } = opResults[0];
console.log(error?.actionIndex); // ex. 0 (as BigNumber)
console.log(error?.message); // ex. "some require failure message"
```

## Signer

Utilities for signing, aggregating and verifying transaction bundles using the
Expand Down
103 changes: 103 additions & 0 deletions contracts/clients/src/OperationResults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { BigNumber, ContractReceipt, utils } from "ethers";
import { ActionData } from "./signer";

type OperationResultError = {
actionIndex: BigNumber;
message: string;
};

export type OperationResult = {
walletAddress: string;
nonce: BigNumber;
actions: ActionData[];
success: Boolean;
results: string[];
error?: OperationResultError;
};

const getError = (
success: boolean,
results: string[],
): OperationResultError | undefined => {
if (success) {
return undefined;
}

// Single event "WalletOperationProcessed(address indexed wallet, uint256 nonce, bool success, bytes[] results)"
// Get the first (only) result from "results" argument.
const [errorResult] = results;
// remove methodId (4bytes after 0x)
const errorArgBytesString = `0x${errorResult.substring(10)}`;
const errorString = utils.defaultAbiCoder.decode(
["string"],
errorArgBytesString,
)[0]; // decoded bytes is a string of the action index that errored.

const splitErrorString = errorString.split(" - ");
if (splitErrorString.length !== 2) {
throw new Error("unexpected error message format");
}

return {
actionIndex: BigNumber.from(splitErrorString[0]),
message: splitErrorString[1],
};
};

export const getOperationResults = (
txnReceipt: ContractReceipt,
): OperationResult[] => {
if (!txnReceipt.events || !txnReceipt.events.length) {
throw new Error(
`no events found in transaction ${txnReceipt.transactionHash}`,

This comment has been minimized.

Copy link
@JohnGuilding

JohnGuilding Oct 26, 2022

Collaborator

It looks like there are a few test cases here that do not have tests written for them. Do we not want to test these different code paths? E.g. lines 51-53, 60-62, 67-69.

I guess a wider question may be what is our philosophy on unit tests and how much do we want to test our code?

);
}

const walletOpProcessedEvents = txnReceipt.events.filter(
(e) => e.event === "WalletOperationProcessed",
);
if (!walletOpProcessedEvents.length) {
throw new Error(
`no WalletOperationProcessed events found in transaction ${txnReceipt.transactionHash}`,
);
}

return walletOpProcessedEvents.reduce<OperationResult[]>(
(opResults, { args }) => {
if (!args) {
throw new Error("WalletOperationProcessed event missing args");
}
const { wallet, nonce, actions: rawActions, success, results } = args;

const actions = rawActions.map(
({
ethValue,
contractAddress,
encodedFunction,
}: {
ethValue: BigNumber;
contractAddress: string;
encodedFunction: string;
}) => ({
ethValue,
contractAddress,
encodedFunction,
}),
);
const error = getError(success, results);

return [
...opResults,
{
walletAddress: wallet,
nonce,
actions,
success,
results,
error,
},
];
},
[],
);
};
4 changes: 4 additions & 0 deletions contracts/clients/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
validateMultiConfig,
} from "./MultiNetworkConfig";

import { OperationResult, getOperationResults } from "./OperationResults";

export * from "./signer";

export {
Expand All @@ -35,6 +37,8 @@ export {
MultiNetworkConfig,
getMultiConfig,
validateMultiConfig,
OperationResult,
getOperationResults,
// eslint-disable-next-line camelcase
VerificationGateway__factory,
VerificationGateway,
Expand Down
4 changes: 3 additions & 1 deletion contracts/contracts/VerificationGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ contract VerificationGateway

// ensure not calling Ownable functions of ProxyAdmin
require((selectorId != Ownable.transferOwnership.selector)
&& (selectorId != Ownable.renounceOwnership.selector));
&& (selectorId != Ownable.renounceOwnership.selector),
"VG: cannot change ownership"
);

wallet.setAnyPending();

Expand Down
16 changes: 7 additions & 9 deletions contracts/test/walletAction-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { parseEther, solidityPack } from "ethers/lib/utils";
import deployAndRunPrecompileCostEstimator from "../shared/helpers/deployAndRunPrecompileCostEstimator";
// import splitHex256 from "../shared/helpers/splitHex256";
import { defaultDeployerAddress } from "../shared/helpers/deployDeployer";
import { getOperationResults } from "../clients/src";

describe("WalletActions", async function () {
if (`${process.env.DEPLOYER_DEPLOYMENT}` === "true") {
Expand Down Expand Up @@ -322,15 +323,12 @@ describe("WalletActions", async function () {
)
).wait();

// Single event "WalletOperationProcessed(address indexed wallet, uint256 nonce, bool success, bytes[] results)"
// Get the first (only) result from "results" argument.
const result = r.events[0].args.results[0]; // For errors this is "Error(string)"
const errorArgBytesString: string = "0x" + result.substring(10); // remove methodId (4bytes after 0x)
const errorString = ethers.utils.defaultAbiCoder.decode(
["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");
const opResults = getOperationResults(r);
expect(opResults).to.have.lengthOf(1);
expect(opResults[0].error.actionIndex.toNumber()).to.eql(1);
expect(opResults[0].error.message).to.eql(
"ERC20: transfer from the zero address",
);

const recipientBalance = await th.testToken.balanceOf(recipient.address);

Expand Down

0 comments on commit 6379006

Please sign in to comment.