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

Viem test #29

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Viem test #29

wants to merge 4 commits into from

Conversation

wuzhong-papermoon
Copy link
Contributor

Viem test re-written.

Rewrite the test for Viem
Copy link
Contributor

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

thanks for working on this! 🙌

there are some changes I suggested in the deploy contracts test that should also be applied to the send transaction tests. let me know if you have any questions! happy to help! 🙂

const privateKey = '0x5fb92d6e98884f76de468fa3f6278f8807c48bebc13595d45af5bdc4da702133'
const rpcUrl = process.env.HTTP_RPC_ENDPOINT

async function compile (){
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
async function compile (){
function compile (){

async function compile (){
var source = fs.readFileSync('contracts/Incrementer.sol', 'utf8');
var input = {
language: 'Solidity',
Copy link
Contributor

Choose a reason for hiding this comment

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

these indents are 4 spaces, but they should be 2. to fix this and all of the other pages at once, you can run npm run prettier and commit the changes


async function deployContract(){
const account = privateKeyToAccount(privateKey);
const rpcUrl = process.env.HTTP_RPC_ENDPOINT;
Copy link
Contributor

Choose a reason for hiding this comment

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

the rpcUrl is already defined on line 10, so we don't need it here

Comment on lines 35 to 45
const account = privateKeyToAccount(privateKey);
const rpcUrl = process.env.HTTP_RPC_ENDPOINT;
const walletClient = createWalletClient({
account,
chain: moonbeamDev,
transport: http(rpcUrl),
});
const publicClient = createPublicClient({
chain: moonbeamDev,
transport: http(rpcUrl),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be moved outside of the deploy function, so then you don't need to create new variables for these again in the describe functions below

Comment on lines 48 to 65
var contractFile = await compile()

const bytecode = contractFile.evm.bytecode.object;
const abi = contractFile.abi;
const _initialNumber = 5;

const contract = await walletClient.deployContract({
abi,
account,
bytecode,
args: [_initialNumber],
});

var transaction = await publicClient.waitForTransactionReceipt({
hash: contract,
});
return transaction.contractAddress
};
Copy link
Contributor

Choose a reason for hiding this comment

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

this code doesn't need to be wrapped in the deploy function instead this function can be:

async function deployContract() {
  const contractFile = await compile();

  const bytecode = contractFile.evm.bytecode.object;
  const abi = contractFile.abi;
  const _initialNumber = 5;

  const contract = await walletClient.deployContract({
    abi,
    account,
    bytecode,
    args: [_initialNumber],
  });

  var transaction = await publicClient.waitForTransactionReceipt({
    hash: contract,
  });
  return transaction.contractAddress;
}


const fromAddressPrivateKey = "0x5fb92d6e98884f76de468fa3f6278f8807c48bebc13595d45af5bdc4da702133"
const fromAddress = "0xf24FF3a9CF04c71Dbc94D0b566f7A27B94566cac"
const toAddress = "0x73328873A6Ac3DFc9455986358F5230cB3B8e92c"
Copy link
Contributor

Choose a reason for hiding this comment

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

the toAddress should be randomly generated, i'm not sure exactly how to do that with viem off-hand but I can look into it if you need any help! 🙂

Comment on lines 6 to 9
const fromAddressPrivateKey = "0x5fb92d6e98884f76de468fa3f6278f8807c48bebc13595d45af5bdc4da702133"
const fromAddress = "0xf24FF3a9CF04c71Dbc94D0b566f7A27B94566cac"
const toAddress = "0x73328873A6Ac3DFc9455986358F5230cB3B8e92c"
const url = process.env.HTTP_RPC_ENDPOINT
Copy link
Contributor

Choose a reason for hiding this comment

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

these variables just should be moved into the describe fn

Comment on lines 19 to 20
const addressFrom = fromAddress;
const addressTo = toAddress;
Copy link
Contributor

Choose a reason for hiding this comment

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

these lines shouldn't be necessary, we can just use one name or the other

});
assert.exists(hash);

// console.log(`Transaction successful with hash: ${hash}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this

const addressTo = toAddress;

const send = async () => {
// console.log(
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this

@eshaben
Copy link
Contributor

eshaben commented Mar 21, 2024

@wuzhong-papermoon - can you go through all of my comments and if you've fixed them, please mark them as resolved, or make the requested changes. And can you also resolve the merge conflicts, please?

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.

2 participants