Skip to content
This repository was archived by the owner on Apr 8, 2022. It is now read-only.

Programmable pool functional tests#86

Merged
altonen merged 7 commits intostagingfrom
pp-validation
Nov 19, 2021
Merged

Programmable pool functional tests#86
altonen merged 7 commits intostagingfrom
pp-validation

Conversation

@altonen
Copy link
Contributor

@altonen altonen commented Nov 3, 2021

This PR adds functional tests for programmable pools and implements an ECTL-type solution familiar from the Qtum whitepaper (the implementation of the ECTL replacement is done only after the testnet).

@muursh
Copy link
Contributor

muursh commented Nov 3, 2021

6.3KB for the largest test contract... Do we have any ideas for the size of a "normal" contract?

@muursh
Copy link
Contributor

muursh commented Nov 3, 2021

Also the CI has run twice and failed in different ways each time...

@altonen
Copy link
Contributor Author

altonen commented Nov 4, 2021

I can't comment on the contract sizes, maybe we could try porting some famous Solidity smart contract to ink! and see how large they are.

As for the test failures, are you able to get it fail consistently on your local machine? If I reduce the block time to 6 seconds and ran the tests, they work almost every time. I haven't tried running them multiple times with 1min block time because they take so long. Anyway, the error message has actually nothing to do with pools but there is a finalization error reported by Grandpa which shuts down one of the nodes and then the tests fail.

@muursh
Copy link
Contributor

muursh commented Nov 4, 2021

As for the test failures, are you able to get it fail consistently on your local machine?

As discussed earlier no. I can run them locally under normal conditions with a 1min/block rate with no issues. The only time I have an issue is if I force them to run in a resource constrained env.

@altonen
Copy link
Contributor Author

altonen commented Nov 4, 2021

@iljakuklic just informed me that the bug (something about chain not being finalized) is reproducible with node-template so I think this is a Parity issue.

@iljakuklic
Copy link
Contributor

@iljakuklic just informed me that the bug (something about chain not being finalized) is reproducible with node-template so I think this is a Parity issue.

Looks very similar to an issue [1] which has been since closed. I am able to reproduce it with node-template semi-reliably.

[1] paritytech/substrate#6727

@muursh
Copy link
Contributor

muursh commented Nov 4, 2021

I am able to reproduce it with node-template semi-reliably

An up to date node-template? What commit?

@iljakuklic
Copy link
Contributor

iljakuklic commented Nov 4, 2021

I am able to reproduce it with node-template semi-reliably

An up to date node-template? What commit?

I used 1ea0883, will try on master tomorrow.

EDIT: 1ea08839a is current master.

@altonen
Copy link
Contributor Author

altonen commented Nov 6, 2021

This time all tests also passed

Instead of allowing the smart contract caller to define the AccountId
of the contract, fetch the value from the ChainExtension's runtime memory.

This allows the programmable pool to force the smart contract to only spend
its own funds.
When PP is constructing the final TX that spends the smart contract's
funds, save the outpoint hashes of the vins to a separate storage.

This indicates to the local node that the smart contract has spent
its funds and when it receives a TX trying to spend CallPPs of a given
smart contract, the outpoint hashes in the witness field can be queried
from this storage and if they are found, the TX is valid (or at least
the local node agrees with the TX and accepts it).
This allows the test framework to query UTXOs of the smart contract.
Prevent a contract from calling itself through the ChainExtension as
the smart contract already has capabilities of calling its own functions
even without the existence of the ChainExtension, makes it harder to
handle contract balances correctly and makes it very easy for the user
to shoot themselves in the foot by accidentally calling the function
that called into the ChainExtension, effectively resulting in infite
recursion (bounded by the amount of gas, obviously).

If this is found to be needed, the issue can be revisited.
If the UTXO transfer from contract to P2PK or contract succeeds,
zero the balance of the smart contract as the contract no longer
holds any funds.
Implement functional tests for PP and create assets directory
which contains all smart contract related files
value=1,
destination=utxo.DestCreatePP(
code=os.path.join(os.path.dirname(__file__), "assets/pooltester.wasm"),
data=[0xed, 0x4b, 0x9d, 0x1b],
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this data?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see it everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Selector (like function ID) for the constructor so the Wasm VM knows which function to call when it's instantiating the smart contract. It was a hard lesson to learn that you must always provide a selector..

Copy link

Choose a reason for hiding this comment

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

These are Selectors pointing to pooltester.json file (i.e "selector": "0xed4b9d1b")

@altonen altonen merged commit a7aaad1 into staging Nov 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants