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

Change Transaction/PendingTransaction behaviors #1480

Merged
merged 12 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Breaking changes

- Improved on parity fix of `Mina.Localblockchain` and`Mina.Network` to make `.send()` and `.wait()` throw an error by default if the transaction was not successful. https://github.com/o1-labs/o1js/pull/1480
- Changed `Transaction.isSuccess` to `Transaction.status` to better represent the state of a transaction.
- `Transaction.safeSend()` and `PendingTransaction.safeWait()` are introduced to return a `IncludedTransaction` or `RejectedTransaction` object without throwing errors.
Copy link
Contributor

Choose a reason for hiding this comment

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

since both of them are in the same release, it would be nice to collapse that changelog with the changes from #1422 mentioned below

for example not have both

  • .send() no longer throws an error ...
  • .send() throws an error by default ...

but just

  • .send() when using Mina.Network is now consistent with LocalBlockchain in that it throws an error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great point! done!

- Fixed parity between `Mina.LocalBlockchain` and `Mina.Network` to have the same behaviors https://github.com/o1-labs/o1js/pull/1422
- Changed the `TransactionId` type to `Transaction`. Additionally added `PendingTransaction` and `RejectedTransaction` types to better represent the state of a transaction.
- `transaction.send()` no longer throws an error if the transaction was not successful for `Mina.LocalBlockchain` and `Mina.Network`. Instead, it returns a `PendingTransaction` object that contains the error. Use `transaction.sendOrThrowIfError` to throw the error if the transaction was not successful.
Expand Down
2 changes: 1 addition & 1 deletion src/examples/zkapps/dex/run-live.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ async function ensureFundedAccount(privateKeyBase58: string) {
}

function logPendingTransaction(pendingTx: Mina.PendingTransaction) {
if (!pendingTx.isSuccess) throw Error('transaction failed');
if (pendingTx.status === 'rejected') throw Error('transaction failed');
console.log(
'tx sent: ' +
(useCustomLocalNetwork
Expand Down
24 changes: 9 additions & 15 deletions src/examples/zkapps/dex/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
(USER_DX * oldBalances.total.lqXY) / oldBalances.dex.X
);
} else {
await expect(tx.sendOrThrowIfError()).rejects.toThrow(
/Update_not_permitted_timing/
);
await expect(tx.send()).rejects.toThrow(/Update_not_permitted_timing/);
}

/**
Expand All @@ -254,14 +252,14 @@ async function main({ withVesting }: { withVesting: boolean }) {
});
await tx.prove();
tx.sign([keys.user2]);
await expect(tx.sendOrThrowIfError()).rejects.toThrow(/Overflow/);
await expect(tx.send()).rejects.toThrow(/Overflow/);
console.log('supplying with insufficient tokens (should fail)');
tx = await Mina.transaction(addresses.user, () => {
dex.supplyLiquidityBase(UInt64.from(1e9), UInt64.from(1e9));
});
await tx.prove();
tx.sign([keys.user]);
await expect(tx.sendOrThrowIfError()).rejects.toThrow(/Overflow/);
await expect(tx.send()).rejects.toThrow(/Overflow/);

/**
* - Resulting operation will overflow the SC’s receiving token by type or by any other applicable limits;
Expand All @@ -280,7 +278,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
);
});
await tx.prove();
await tx.sign([feePayerKey, keys.tokenY]).sendOrThrowIfError();
await tx.sign([feePayerKey, keys.tokenY]).send();
console.log('supply overflowing liquidity');
await expect(async () => {
tx = await Mina.transaction(addresses.tokenX, () => {
Expand All @@ -291,7 +289,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
});
await tx.prove();
tx.sign([keys.tokenX]);
await tx.sendOrThrowIfError();
await tx.send();
}).rejects.toThrow();

/**
Expand All @@ -318,7 +316,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
dex.supplyLiquidity(UInt64.from(10));
});
await tx.prove();
await expect(tx.sign([keys.tokenX]).sendOrThrowIfError()).rejects.toThrow(
await expect(tx.sign([keys.tokenX]).send()).rejects.toThrow(
/Update_not_permitted_balance/
);

Expand All @@ -345,9 +343,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
});
await tx.prove();
tx.sign([keys.user]);
await expect(tx.sendOrThrowIfError()).rejects.toThrow(
/Source_minimum_balance_violation/
);
await expect(tx.send()).rejects.toThrow(/Source_minimum_balance_violation/);

// another slot => now it should work
Local.incrementGlobalSlot(1);
Expand Down Expand Up @@ -456,7 +452,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
});
await tx.prove();
tx.sign([keys.user, keys.user2]);
await expect(tx.sendOrThrowIfError()).rejects.toThrow(
await expect(tx.send()).rejects.toThrow(
/Account_balance_precondition_unsatisfied/
);

Expand Down Expand Up @@ -491,9 +487,7 @@ async function main({ withVesting }: { withVesting: boolean }) {
dex.redeemLiquidity(UInt64.from(1n));
});
await tx.prove();
await expect(tx.sign([keys.user2]).sendOrThrowIfError()).rejects.toThrow(
/Overflow/
);
await expect(tx.sign([keys.user2]).send()).rejects.toThrow(/Overflow/);
[oldBalances, balances] = [balances, getTokenBalances()];

/**
Expand Down
18 changes: 9 additions & 9 deletions src/examples/zkapps/dex/upgradability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ async function atomicActionsTest({ withVesting }: { withVesting: boolean }) {
});
await tx.prove();

await expect(
tx.sign([feePayerKey, keys.dex]).sendOrThrowIfError()
).rejects.toThrow(/Cannot update field 'delegate'/);
await expect(tx.sign([feePayerKey, keys.dex]).send()).rejects.toThrow(
/Cannot update field 'delegate'/
);

console.log('changing delegate permission back to normal');

Expand Down Expand Up @@ -185,9 +185,9 @@ async function atomicActionsTest({ withVesting }: { withVesting: boolean }) {
fieldUpdate.requireSignature();
});
await tx.prove();
await expect(
tx.sign([feePayerKey, keys.dex]).sendOrThrowIfError()
).rejects.toThrow(/Cannot update field 'delegate'/);
await expect(tx.sign([feePayerKey, keys.dex]).send()).rejects.toThrow(
/Cannot update field 'delegate'/
);

/**
* # Atomic Actions 3
Expand Down Expand Up @@ -461,9 +461,9 @@ async function upgradeabilityTests({ withVesting }: { withVesting: boolean }) {
modifiedDex.deploy(); // cannot deploy new VK because its forbidden
});
await tx.prove();
await expect(
tx.sign([feePayerKey, keys.dex]).sendOrThrowIfError()
).rejects.toThrow(/Cannot update field 'verificationKey'/);
await expect(tx.sign([feePayerKey, keys.dex]).send()).rejects.toThrow(
/Cannot update field 'verificationKey'/
);

console.log('trying to invoke modified swap method');
// method should still be valid since the upgrade was forbidden
Expand Down
4 changes: 2 additions & 2 deletions src/examples/zkapps/hello-world/run-live.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ let transaction = await Mina.transaction(
transaction.sign([senderKey, zkAppKey]);
console.log('Sending the transaction.');
let pendingTx = await transaction.send();
if (pendingTx.hash !== undefined) {
if (pendingTx.status === 'pending') {
console.log(`Success! Deploy transaction sent.
Your smart contract will be deployed
as soon as the transaction is included in a block.
Expand All @@ -77,7 +77,7 @@ transaction = await Mina.transaction({ sender, fee: transactionFee }, () => {
await transaction.sign([senderKey]).prove();
console.log('Sending the transaction.');
pendingTx = await transaction.send();
if (pendingTx.hash !== undefined) {
if (pendingTx.status === 'pending') {
console.log(`Success! Update transaction sent.
Your smart contract state will be updated
as soon as the transaction is included in a block.
Expand Down
16 changes: 8 additions & 8 deletions src/examples/zkapps/hello-world/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ txn = await Mina.transaction(feePayer1.publicKey, () => {
AccountUpdate.fundNewAccount(feePayer1.publicKey);
zkAppInstance.deploy();
});
await txn.sign([feePayer1.privateKey, zkAppPrivateKey]).sendOrThrowIfError();
await txn.sign([feePayer1.privateKey, zkAppPrivateKey]).send();

const initialState =
Mina.getAccount(zkAppAddress).zkapp?.appState?.[0].toString();
Expand All @@ -45,7 +45,7 @@ txn = await Mina.transaction(feePayer1.publicKey, () => {
zkAppInstance.update(Field(4), adminPrivateKey);
});
await txn.prove();
await txn.sign([feePayer1.privateKey]).sendOrThrowIfError();
await txn.sign([feePayer1.privateKey]).send();

currentState = Mina.getAccount(zkAppAddress).zkapp?.appState?.[0].toString();

Expand All @@ -70,7 +70,7 @@ try {
zkAppInstance.update(Field(16), wrongAdminPrivateKey);
});
await txn.prove();
await txn.sign([feePayer1.privateKey]).sendOrThrowIfError();
await txn.sign([feePayer1.privateKey]).send();
} catch (err: any) {
handleError(err, 'Account_delegate_precondition_unsatisfied');
}
Expand All @@ -91,7 +91,7 @@ try {
zkAppInstance.update(Field(30), adminPrivateKey);
});
await txn.prove();
await txn.sign([feePayer1.privateKey]).sendOrThrowIfError();
await txn.sign([feePayer1.privateKey]).send();
} catch (err: any) {
handleError(err, 'assertEquals');
}
Expand All @@ -118,7 +118,7 @@ try {
}
);
await txn.prove();
await txn.sign([feePayer1.privateKey]).sendOrThrowIfError();
await txn.sign([feePayer1.privateKey]).send();
} catch (err: any) {
handleError(err, 'assertEquals');
}
Expand All @@ -134,7 +134,7 @@ txn2 = await Mina.transaction({ sender: feePayer2.publicKey, fee: '2' }, () => {
zkAppInstance.update(Field(16), adminPrivateKey);
});
await txn2.prove();
await txn2.sign([feePayer2.privateKey]).sendOrThrowIfError();
await txn2.sign([feePayer2.privateKey]).send();

currentState = Mina.getAccount(zkAppAddress).zkapp?.appState?.[0].toString();

Expand All @@ -151,7 +151,7 @@ txn3 = await Mina.transaction({ sender: feePayer3.publicKey, fee: '1' }, () => {
zkAppInstance.update(Field(256), adminPrivateKey);
});
await txn3.prove();
await txn3.sign([feePayer3.privateKey]).sendOrThrowIfError();
await txn3.sign([feePayer3.privateKey]).send();

currentState = Mina.getAccount(zkAppAddress).zkapp?.appState?.[0].toString();

Expand All @@ -174,7 +174,7 @@ try {
}
);
await txn4.prove();
await txn4.sign([feePayer4.privateKey]).sendOrThrowIfError();
await txn4.sign([feePayer4.privateKey]).send();
} catch (err: any) {
handleError(err, 'assertEquals');
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/account-update.unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function createAccountUpdate() {
AccountUpdate.fundNewAccount(feePayer);
});
tx.sign();
await expect(tx.sendOrThrowIfError()).rejects.toThrow(
await expect(tx.send()).rejects.toThrow(
'Check signature: Invalid signature on fee payer for key'
);
}
2 changes: 1 addition & 1 deletion src/lib/caller.unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ let tx = await Mina.transaction(privateKey, () => {
});

// according to this test, the child doesn't get token permissions
await expect(tx.sendOrThrowIfError()).rejects.toThrow(
await expect(tx.send()).rejects.toThrow(
'can not use or pass on token permissions'
);
Loading
Loading