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

examples/onchain-signer/test: Add support for Sapphire Testnet/Mainnet #208

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

matevz
Copy link
Member

@matevz matevz commented Oct 18, 2023

This PR makes examples/onchain-signer tests actually pass on sapphire-testnet.

Merge after #175 since we use the wrapped provider for gasless tx.

@matevz matevz added the js label Oct 18, 2023
@CedarMist CedarMist force-pushed the matevz/fix/onchain-signer-networks branch from e36ac06 to be85b2c Compare October 19, 2023 01:13
@CedarMist
Copy link
Member

I made some minor changes to the Gasless contract to simplify it, the tests pass locally and on testnet.

There are 2 problems:

  1. When deploying via CI I get, Error: cannot estimate gas; transaction may fail or may require manual gas limit
  2. I realized my changes would require changes to https://docs.oasis.io/dapp/sapphire/gasless

To add back in the setKeypair function so it matches the documentation.

@CedarMist CedarMist added p:1 Priority: high s:underway Status: a patch is in progress contracts Pull requests that update sapphire-contracts labels Oct 19, 2023
@CedarMist CedarMist self-requested a review October 19, 2023 13:59
@CedarMist CedarMist force-pushed the matevz/fix/onchain-signer-networks branch from 4c8f024 to f5c3a72 Compare October 19, 2023 14:00
@CedarMist CedarMist merged commit c4b8e09 into main Oct 19, 2023
17 checks passed
@CedarMist CedarMist deleted the matevz/fix/onchain-signer-networks branch October 19, 2023 14:45
Copy link
Member Author

@matevz matevz left a comment

Choose a reason for hiding this comment

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

I know this was already merged, just leaving some comments for future.

// Proxy for gasless transaction.
contract Gasless {
EthereumKeypair private kp;

function setKeypair(EthereumKeypair memory keypair) external payable {
constructor (EthereumKeypair memory keypair) payable {
Copy link
Member Author

Choose a reason for hiding this comment

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

Contract create transactions are usually unencrypted for verification, so that's why I decided to have the setKeypair setter.

nonce: ethers.provider.getTransactionCount(wallet1.address),
}),
).not.to.be.reverted;
// NOTE can be done by the contract with EthereumUtils.generateKeypair()
Copy link
Member Author

Choose a reason for hiding this comment

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

// NOTE: Can be done by the contract with EthereumUtils.generateKeypair().

const tx = await gasless.makeProxyTx(commentBox.address, innercall);

const plainResp = await gasless.provider.sendTransaction(tx);
const receipt = await ethers.provider.waitForTransaction(plainResp.hash);
// TODO: https://github.com/oasisprotocol/sapphire-paratime/issues/179
Copy link
Member Author

Choose a reason for hiding this comment

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

Not relevant anymore.

@@ -9,22 +9,15 @@ struct EthereumKeypair {
uint64 nonce;
}

struct EthTx {
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we rename EIP155Signer.EthTx to EIP155Signer.Tx or EIP155Signer.Transaction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Pull requests that update sapphire-contracts p:1 Priority: high s:underway Status: a patch is in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants