oasisprotocol/sapphire-paratime

Hardhat Signature problem - state doesn't change

Formator opened this issue · 30 comments

I have a problem with hardhat testing contracts on the sapphire network.
I can't change the state of the already deployed smart contract with a combination when to use or when not to use sapphire.wrap.

To demonstrate my issue I have forked this repo and added some changes and scripts.

Before running please set env PRIVATE_KEY_TESTER beside PRIVATE_KEY.

This is the console output that I'm getting from \examples\hardhat-boilerplate\scripts\deploy-transfer.js

Deploying the contracts with the account: 0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Account balance: 6417194500000000000
Token address: 0x034BA58e63898205dCCf2dc57578Bb8fb930Bf08
Fauceting tester account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8

And this is the console output that I'm getting from \examples\hardhat-boilerplate\scripts\test.js

Token address: 0x034BA58e63898205dCCf2dc57578Bb8fb930Bf08
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 3995157700000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Tester account token balance before transfer: 10
Tester account token balance after transfer: 10
3 - Check if tester account can transfer 10 token to deployer account
Tester account token balance before transfer: 10
Tester account token balance after transfer: 10

As you can see in test script I couldn't change the state of the smart contract even from the deployer signer which has worked on deploy-transfer script.

Same contract, and same test scenario works from the frontend example.

Thanks for the reproducible test case. I think the reason you're seeing this is because all of the transactions revert because an unmodified hardhat node is not able to decode the encrypted payloads understood by Sapphire. I think that I have a fix for you. Please allow me a few hours!

@nhynes No problem I will wait for your fix, thank you!

Took a little longer than expected, but I think that #108 will do the trick. The idea there is that when you run hardhat node, if confidential: true is present in your hardhat network config, you'll end up starting a Sapphire-compatible version of ethereumjs-vm. Right now, only calls and transactions are supported, and we're working on adding the precompiles like RNG. Please let us know if there are any that you need so that we can prioritize them!

It sounds like we should make the confidential default to be false in our version of hardhat node?

It sounds like we should make the confidential default to be false in our version of hardhat node?

No, it's fine that it's default true. Our version of hardhat node doesn't get started unless the config is confidential already. hardhat test might need the same treatment.

@nhynes Thank you very much. I have tested your branch but without luck, here are my steps:

  • Checkout your branch
  • In /sapphire-paratime/integrations/hardhat I have run yarn install, yarn add @oasislabs/hardhat and yarn build
  • Then I have copy sapphire-paratime/integrations/hardhat/dist/src/ to my local test branch \examples\hardhat-boilerplate\node_modules\@oasisprotocol\sapphire-hardhat\ and \sapphire-paratime\integrations\hardhat\dist\test\ to \examples\hardhat-boilerplate\node_modules\@oasisprotocol\sapphire-hardhat\test\
  • Then I made changes in my local test \examples\hardhat-boilerplate\hardhat.config.js:
networks: {
    hardhat: {
      chainId: 1337, // We set 1337 to make interacting with MetaMask simpler
      confidential: true,
    },
    sapphire: {
      confidential: true,
      url: "https://testnet.sapphire.oasis.dev",
      accounts: process.env.PRIVATE_KEY && process.env.PRIVATE_KEY_TESTER
        ? [process.env.PRIVATE_KEY, process.env.PRIVATE_KEY_TESTER]
        : process.env.PRIVATE_KEY ? [process.env.PRIVATE_KEY] : [],
      chainId: 0x5aff,
    },
  }
  • Run the command: hardhat run scripts/deploy-transfer.js and get this error:
Deploying the contracts with the account: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Account balance: 10000000000000000000000
Error: CBOR decode error: too many terminals, data makes no sense
    at Object.decode (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\cborg\cjs\lib\decode.js:133:11)
    at VM._runTx (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\@oasislabs\ethereumjs-vm\src\runTx.ts:326:49)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at runNextTicks (node:internal/process/task_queues:65:3)    at listOnTimeout (node:internal/timers:528:9)
    at processTimers (node:internal/timers:502:7)
    at VM.runTx (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\@oasislabs\ethereumjs-vm\src\runTx.ts:118:20)
    at HardhatNode._runTxAndRevertMutations (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\hardhat\src\internal\hardhat-network\provider\node.ts:2428:14)
    at HardhatNode._runInPendingBlockContext (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\hardhat\src\internal\hardhat-network\provider\node.ts:2205:14)
    at HardhatNode.estimateGas (D:\VSGIT\Spartan\sapphire-paratime-formator\examples\hardhat-boilerplate\node_modules\hardhat\src\internal\hardhat-network\provider\node.ts:803:20)
  • Running the command: hardhat run scripts/deploy-transfer.js --network sapphire and hardhat run scripts/test.js --network sapphire doesn't change the described issue.

Sorry, but maybe I didn't clearly describe the problem that I have, comes from deploying and testing on sapphire network, not on hardhat

Ah, my mistake. For some reason I thought you were developing locally. Let me run your reproducible example and fix it. Next time I post back, it should "just work" :) Thanks for your patience!

@nhynes Thank you

@nhynes Are you still on this issue?

Hey @Formator, I scheduled looking into this for this week. I'm sorry that it's taking so long. There's quite a bit to be done in time for big events like EthDenver, but I really want to solve your issue, so it's in progress–just a bit slowly.

Hi, @nhynes thank you for your reply, I understand, no problem I will wait.

Hey, @nhynes any news about our issue, it is kind of a blocker for deploying from hardhat, so we would be very grateful if we could solve this issue to deploy our solution on-chain.

Hi @nhynes, sorry to bother you again but nothing has changed, or I can't test it properly on my local.

Now I have improved my issue replication, maybe this will help you understand what the problem is.

So in my setup, I have pulled the last sources from this repo and:

  • Compiled sapphire-paratime\integrations\hardhat\ and copy the content of dist\src\ over sapphire-hardhat-state-issue\node_modules@oasisprotocol\sapphire-hardhat\
  • Compiled sapphire-paratime\clients\js\ and copy the content of \lib\ over \sapphire-hardhat-state-issue\node_modules@oasisprotocol\sapphire-paratime\lib\ and \src\ over \sapphire-hardhat-state-issue\node_modules@oasisprotocol\sapphire-paratime\src\

Then I have done all steps from my issue Readme.

Test output

Token address: 0xe01dCBaBe0FFB65b29df36C39FA28822517ceb49
Token deployer was:  0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 1992512000000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Deployer account token balance before transfer: 1000000
Tester account token balance before transfer: 0
Deployer account token balance after transfer: 1000000
Tester account token balance after transfer: 0
ERROR: Tester account token balance not changed!!
3 - Check if tester account can transfer 10 token to deployer account
Tester don't have enough tokens to send. Balance: 0

As you can see from my test, I can't transfer tokens from one account to another.
Please note that all deployment and tests are done on Sapphire Testnet!

Does the balance check still report the same if you query after the next block?

@kostko Even if I repeat the same test on the same Token address after one hour I will get the same balances.

I can have a look and try to repro the issue.

@xmzheng Thank you

Please take a look test.js I have included comments, to clarify the test scenario.

3 - Check if tester account can transfer 10 token to deployer account
Deployer account token balance before transfer: 999900

yeah, the token transfer transaction can happen successfully, but it is confusing when to use wrap or not.

let me digger a little while and update you more.

the main reason is that you need to wait the transaction to be mined for testnet, so I added await sleep(15000) after the transaction, which makes the script work.

@Formator if you gave the permission to me, I can push my codes to your repo:

remote: Permission to Formator/sapphire-hardhat-state-issue.git denied to xmzheng.

@xmzheng I have added you to my repo.

@xmzheng Normally I'm using await tx2.wait(2); to wait for two confirmations, but even with that "2. Check .." doesn't work on my side.

@xmzheng Thank you but I still get the same results as before.

/home/formator/.nvm/versions/node/v16.19.0/bin/npx hardhat run scripts/test.js --network sapphire
tester:  SignerWithAddress {_isSigner: true, address: '0xC88309139e5809f4E736D700bB8Aca31540C67d8', _signer: JsonRpcSigner, provider: EthersProviderWrapper}
Token address: 0x29512C25ac8d5bE86FD83bee380EDAc5a726947C
Token deployer was:  0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 1992512000000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Deployer account token balance before transfer: 1000000
Tester account token balance before transfer: 0
before transaction
transaction completed
Deployer account token balance after transfer: 1000000
Tester account token balance after transfer: 0
ERROR: Tester account token balance not changed!!
3 - Check if tester account can transfer 10 token to deployer account
Tester don't have enough tokens to send. Balance: 0
  1. Fresh clone this repo and inside /sapphire-paratime/integrations/hardhat execute yarn install, select @oasisprotocol/sapphire-paratime: 1.0.15 and for the finish execute yarn build
  2. Moving on to my repo. I have Change dependencies "@oasisprotocol/sapphire-hardhat": "file:../sapphire-paratime/integrations/hardhat"
  3. Delete all node_models and reinstall with yarn install
  4. Redeployed my smart contract
  5. Rerun test script

Did I miss some dependencies?

@Formator it works for me if I remove the extra sapphire.wrap(...) on the signers and add an extra block of wait time. Given that the sapphire hardhat plugin is doing the wrapping automatically, such wrapping should not be needed and it seems it results in some strange issues.

E.g. try the following patch:

diff --git a/scripts/test.js b/scripts/test.js
index 6e9c449..d8323fe 100644
--- a/scripts/test.js
+++ b/scripts/test.js
@@ -30,10 +30,10 @@ async function main() {
   const [deployer, tester] = await ethers.getSigners();
 
   const deployerTokenReadInstance = await token.connect(deployer);
-  const deployerTokenWriteInstance = await token.connect(sapphire.wrap(deployer));
+  const deployerTokenWriteInstance = await token.connect(deployer);
 
   const testerTokenReadInstance = await token.connect(tester);
-  const testerTokenWriteInstance = await token.connect(sapphire.wrap(tester));
+  const testerTokenWriteInstance = await token.connect(tester);
 
   console.log("Token address:", token.address);
   console.log("Token deployer was: ", await deployer.getAddress());
@@ -69,7 +69,7 @@ async function main() {
     // Send some token to the tester account
     // ISSUE: transfer transaction below work without revert, but blockchain state will not be changed (balanceOf call for tester return same as before)
     const tx2 = await deployerTokenWriteInstance.transfer(tester.address, 100);
-    await tx2.wait();
+    await tx2.wait(2);
     testerTokenBalanceAfter = await deployerTokenReadInstance.balanceOf(tester.address);
     deployerTokenBalanceAfter = await deployerTokenReadInstance.balanceOf(deployer.address);
     console.log("Deployer account token balance after transfer:", deployerTokenBalanceAfter.toString());
@@ -92,7 +92,7 @@ async function main() {
     // Send some token to the deployer account
     // ISSUE: transfer transaction below work without revert, but blockchain state will not be changed (balanceOf call for tester return same as before)
     const tx3 = await testerTokenWriteInstance.transfer(deployer.address, 10);
-    await tx3.wait();
+    await tx3.wait(2);
     testerTokenBalanceAfter = await deployerTokenReadInstance.balanceOf(tester.address);
     deployerTokenBalanceAfter = await deployerTokenReadInstance.balanceOf(deployer.address);
     console.log("Deployer account token balance after transfer:", deployerTokenBalanceAfter.toString());

Oh I see, it seems we properly handle double wrapping of a provider but if one passes a signer that has a provider, we wrap again.

@kostko Thank you, your patch did work in my commit, so transfers now work after two confirmations.

tester:  SignerWithAddress {_isSigner: true, address: '0xC88309139e5809f4E736D700bB8Aca31540C67d8', _signer: JsonRpcSigner, provider: EthersProviderWrapper}
Token address: 0x29512C25ac8d5bE86FD83bee380EDAc5a726947C
Token deployer was:  0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 11992512000000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Deployer account token balance before transfer: 1000000
Tester account token balance before transfer: 0
before transaction
transaction completed
Deployer account token balance after transfer: 999900
Tester account token balance after transfer: 100
3 - Check if tester account can transfer 10 token to deployer account
Deployer account token balance before transfer: 999900
Tester account token balance before transfer: 100
Deployer account token balance after transfer: 999910
Tester account token balance after transfer: 90

Next issue is that I can't get balance from testerTokenReadInstance, it only work from deployerTokenReadInstance.
My commit and error that I get:

tester:  SignerWithAddress {_isSigner: true, address: '0xC88309139e5809f4E736D700bB8Aca31540C67d8', _signer: JsonRpcSigner, provider: EthersProviderWrapper}
Token address: 0x29512C25ac8d5bE86FD83bee380EDAc5a726947C
Token deployer was:  0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 11979414400000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Error: missing revert data in call exception; Transaction reverted without a reason string [ See: https://links.ethers.org/v5-errors-CALL_EXCEPTION ] (data="0x", transaction={"from":"0xC88309139e5809f4E736D700bB8Aca31540C67d8","to":"0x29512C25ac8d5bE86FD83bee380EDAc5a726947C","data":"0x70a08231000000000000000000000000c88309139e5809f4e736d700bb8aca31540c67d8","accessList":null}, error={"name":"ProviderError","_stack":"ProviderError: invalid signed simulate call query: signer != caller\n    at HttpProvider.request 

@kostko Thank you, your patch did work in my commit, so transfers now work after two confirmations.

tester:  SignerWithAddress {_isSigner: true, address: '0xC88309139e5809f4E736D700bB8Aca31540C67d8', _signer: JsonRpcSigner, provider: EthersProviderWrapper}
Token address: 0x29512C25ac8d5bE86FD83bee380EDAc5a726947C
Token deployer was:  0x84373E739d2c50eE55EF8a3AF7238f77804f83bF
Testing the contracts with the account: 0xC88309139e5809f4E736D700bB8Aca31540C67d8
1 - Check if tester account has enough ether
Tester account balance: 11992512000000000000
2 - Check if tester account has at least 100 tokens if he has't, send them from deployer account
Deployer account token balance before transfer: 1000000
Tester account token balance before transfer: 0
before transaction
transaction completed
Deployer account token balance after transfer: 999900
Tester account token balance after transfer: 100
3 - Check if tester account can transfer 10 token to deployer account
Deployer account token balance before transfer: 999900
Tester account token balance before transfer: 100
Deployer account token balance after transfer: 999910
Tester account token balance after transfer: 90

Great, I'm glad @kostko's patch helped resolve the issue.

Next issue is that I can't get balance from testerTokenReadInstance, it only work from deployerTokenReadInstance.

@Formator, can I ask you to file this next issue as a separate issue?

Next issue is that I can't get balance from testerTokenReadInstance, it only work from deployerTokenReadInstance.

@Formator, can I ask you to file this next issue as a separate issue?

This is now tracked here #123.