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

Update tests to use account.signTransaction #571

Open
wants to merge 3 commits into
base: feat/next
Choose a base branch
from

Conversation

danielailie
Copy link
Contributor

No description provided.


const userVerifier = new UserVerifier(new UserPublicKey(wallets.alice.address.getPublicKey()));
const userVerifier = new UserVerifier(alice.publicKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use account.verify()?

Now I see, that we don't have separate verify_transaction and verify_message:

How about having so?

@@ -373,7 +365,7 @@ describe("ApiNetworkProvider Tests", function () {
data: new Uint8Array(Buffer.from("dummy@05")),
});
transaction.nonce = nonce + 1n;
transaction.signature = await bob.signer.sign(transactionComputer.computeBytesForSigning(transaction));
transaction.signature = bob.signTransaction(transaction);
Copy link
Contributor

Choose a reason for hiding this comment

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

await?

@@ -298,7 +289,7 @@ describe("ProxyNetworkProvider Tests", function () {
let txOnNetwork = await proxy.simulateTransaction(transaction);
assert.deepEqual(txOnNetwork.status, new TransactionStatus("success"));

transaction.signature = await bob.signer.sign(transactionComputer.computeBytesForSigning(transaction));
transaction.signature = bob.signTransaction(transaction);
Copy link
Contributor

Choose a reason for hiding this comment

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

await - here and in other places, as needed?

Comment on lines +271 to +275
deployTransaction.nonce = alice.getNonceThenIncrement();

deployTransaction.signature = await alice.signer.sign(
Buffer.from(transactionComputer.computeBytesForSigning(deployTransaction)),
);
deployTransaction.signature = alice.signTransaction(deployTransaction);

const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

if you use getNonceThenIncrement() and then try to compute the contract's address using alice.nonce you'll have a higher nonce than the one on the transaction, thus computing an invalid address.

Comment on lines +359 to +363
deployTransaction.nonce = alice.getNonceThenIncrement();

deployTransaction.signature = await alice.signer.sign(
Buffer.from(transactionComputer.computeBytesForSigning(deployTransaction)),
);
deployTransaction.signature = alice.signTransaction(deployTransaction);

const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above


const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment with the higher nonce and wrong contract address.


const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue here


const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue here, as well


const contractAddress = SmartContract.computeAddress(alice.address, alice.account.nonce);
alice.account.incrementNonce();
const contractAddress = SmartContract.computeAddress(alice.address, alice.nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue here.

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