arkeonetwork / arkeo Goto Github PK

create stats endpoint in router and return json obj with needed stats.

#74 was done to allow for easy testing but we should remove all such functionality prior to launch and or final audit.

Background

Currently users should not be able to cancel a PayAsYouGo contract since the provider needs to collect funds from yet to be settled calls.

This can be bypassed if a contract has a delegate and the user constructs the payload without a client.

additionally a contract with a delegate can be cancelled by anyone as demonstrated here -

master...dos-vector

We need to resolve these issue.

AC

1. Ensure a PayAsYouGo contract with a delegate cannot be cancelled even with msg.client is ""
2. Ensure any contract with a delegate cannot be cancelled even with msg.client is ""
3. Add tests for all of these.

The behaviour is after sentinel is running , open a contract , even the contract still have enough deposit , sentinel still return http.StatusPaymentRequired

The root cause:
Currently there are two places to pull a contract from arkeod

1. when the request contract is not in memory cache , it send a request to arkeod to fetch it, this is passive , pull based
2. sentinel is using websocket to monitor the events happened on arkeod , when it read an open_contract event , it will create the contract , and save it to in memory cache. this is like arkeod push the contract to sentinel

When a request comes in , it check in memory store to find the contract , if it already exist , then it will not trigger 1).

However , on step 2) , open_contract it doesn't have Deposit value , which result the contract which stored in memory have Deposit value 0

As a result of that , the request will be rejected as not enough payment

Code audit prior to launch

Background

Once the airdrop has been completed, we should transfer all tokens back to the reserve and prune all records from the store.

This should be added to the EndBlocker and check if the airdrop time is over.

Some good examples of how this should work:

https://github.com/osmosis-labs/osmosis/blob/v4.2.0/x/claim/abci.go
and
https://github.com/osmosis-labs/osmosis/blob/e79fc9f47f2bd4518d9e16567a369c25a0820fa6/x/claim/keeper/claim.go#L31

Acceptance Criteria

• endblocker checks for airdrop expiration
• once airdrop is over it transfers unclaimed tokens to reserve
• tests added to show this functionality working correctly.

When we import a genesis state, we do not create the expiration and user sets for contracts leading to unexpected behaviors.

AC

1. fix import genesis to create the needed sets in the KV store.

Background

After phase 1 testing has been completed as outlined in #59 we can proceed to phase 2 involving multiple nodes.

The owner of this ticket will be responsible for the following:

1. Drafting test suite that includes opening and closing contracts across different nodes. We should also include tests for some of the airdrop functionality. Would be good to get approval from @cbarraford and @johnnyluo on the suite.
2. Enlisting 3-4 node providers (foundation + other community members who are willing to participate).
3. Supporting node providers and testers to shepherd them through to completion of phase 2
4. Any bugs found get captured in tickets and or resolved.

Open Questions

1. should we include delegations to validators and potentially some governance type actions as well ?
2. Should we include an attempt to upgrade the chain during this process?

Test suite suggestions

1. should include a provider with and without a free tier.

Background

This is mainly just a nice to have, but currently the ClaimModule doesn't make use of the nice typed errors like the main Arkeo Module

We could clean this up by following the same pattern and also improve the claim modules tests to be looking for a specific error vs any error

Acceptance Criteria

1. Claim module has reusable error types
2. error returns in the Claim module wrap these types where applicable
3. Claim module tests are updated to expect specific error types, vs generic errors.

It appears that we are using two different key structures for claims

this leads to the ability to re-use nonces and replay attacking a provider.

AC

• add tests to ensure no replay attacks are possible, calling paidTier with same nonce should fail.

Background

The cosmos sdk offers the crisis module to register invariant states that should halt the chain

https://docs.cosmos.network/main/modules/crisis

We should draft all invariant states based and implement this fail safe mechanism

AC

1. draft list of all invariant states that may exist for arkeo
2. implement crisis module with tests

Background

Currently in our private testnet environment we are using a production chain id and address prefix. This could become problematic when we open it to a broader audience due to replay attacks or user error

AC

• Create needed config changes for testnet to utilize a distinct chain id and address prefix

Currently when attempting to sign with a key in the backend os curleo fails to find the keys for some reason.

(base) ➜  arkeonetwork curleo  --keyring-backend os -u jon  -data '{ "jsonrpc": "2.0", "method": "block", "params": ["14218072"], "id": 1 }' -H "text/plain" http://testnet-sentinel.arkeo.shapeshift.com:3636/gaia-mainnet-rpc-archive 

2023/02/24 13:57:34 jon.info: key not found [cosmos/[email protected]/crypto/keyring/keyring.go:489]

(base) ➜  arkeonetwork arkeod keys list --keyring-backend os                                                                                                                                                                           
- address: arkeo1wk2sg3yxwur5xsnwmvxzrwutuf8tkg8h09kxkg
  name: jon
  pubkey: '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"Ar6CeK0Vvfm8dbLjDdzAfGxQsbGDa5mjgIVhoXSNGtER"}'
  type: local
- address: arkeo1wp4gaass27d8ghy9p3wcxpguvyy964ky7azsnz
  name: testnet-bob
  pubkey: '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"AiYI6or1FhVmNrmZnppCzQMSDoEzy0zdhuNDAI0AWoph"}'
  type: local

The same key, when moved to a file, works as expected.

(base) ➜  arkeonetwork curleo  --keyring-backend file -u jon  -data '{ "jsonrpc": "2.0", "method": "block", "params": ["14218072"], "id": 1 }' -H "text/plain" http://testnet-sentinel.arkeo.shapeshift.com:3636/gaia-mainnet-rpc-archive 

Enter keyring passphrase:
invoking Sign...
Enter keyring passphrase:
Signed successfully
making POST request to http:/

https://github.com/arkeonetwork/arkeo/blob/master/sentinel/memstore.go

Current memstore is use a simple map , and it has been used in an multi-thread environment, need to use lock

Background

Currently a provider has no easy way of not providing one of the contract types. They could set arbitrarily high prices but there should be an easy way for a provider to not support all the contract types.

AC

1. Create boolean flag for the provider to be able to designate their support of PayAsYouGo and Subscription contract types
2. Add tests to ensure that contracts cannot be open against these providers when the flags are false.

Looks like we can self serve - https://github.com/chainapsis/keplr-chain-registry
we should also add the thorchain BIP as secondary - an example is here
https://github.com/chainapsis/keplr-chain-registry/blob/ba99c5ce9c76b5eeb56555fb211ec9e34db53dd3/cosmos/mars.json#L16

Background

Currently the active contract endpoint just iterates the mem store to find a match for an active contract, this could be greatly improved to index contracts by spender. A spender could have multiple contracts in varying states in the mem store, so we would need to handle that accordingly.

Finally, if we cannot find an active contract, we should call arkeo itself to see if there is one we missed in our cache.

Acceptance Criteria

1. fix mem store cache to a more efficient lookup for a user's active contracts
2. add functionality to find the active contract for a specific spender, provider, and chain
3. add fallback for a cache miss to hit the arkeo node as a last resort.
4. extend and clean up tests for these scenarios

Background

when we airdrop arkeo to thorchain users, their claims will be available to them on the account that is derived using the thorchain standard derivation path. This will be different than the standard arkeo derivation path that will use coin type 118. In order to provide thorchain users with an easy way to get back to the standard path, we want to make specific ClaimRecords have the ability to transfered once to a different arkeo adress, and then combined with any ClaimRecord that exists for that arkeo address.

This should only be able to do be done once, and only for ClaimRecords that are designated as transferrable, so that others cannot transfer their claims.

for more information on the airdrop spec, see the docs here

Acceptance Criteria

1. Modify the ClaimRecord protobuf to include an is_transferable flag.
2. Use ignite to scaffold a new msg called TransferClaim(toAddress)
3. Build out the handler for TransferClaim and include a validateBasic function for it
4. TransferClaim should validate the caller has a valid claim and the airdrop has not ended and the claim is transferable, then it should merge the claim with any claim the toAddress has (merge functionality is already built for ETH claims, re-use that). The new claim/merged claim should not be transferable.
5. Add tests for all functionality and validate basic. Test should cover a scenario where the claim is not transferable, the claimee has no claim, the toAddress has no claim to merge with, the toAddress has an existing claim to merge with, the claimee tries to transfer it twice.

Background

Currently contracts and contract expiration sets are never pruned once they have completed their lifecycles. We should resolve this to avoid the chain growing in size with uneeded state.

AC

1. Once a contract is expired and completely settled, we should remove it from the KV store.
2. Once a contract expiration set has reach the expiration block and been processed, we should remove from the KV store
3. Ensure all tests still pass and add new tests to ensure the contracts and sets dont exist after they are dead.

We should add sanity checks to the following functions

• validateAirdropStartTime
• validateDurationOfDecay
• validateDurationUntilDecay

Background

Currently, the intended mechanism for the airdrop is that all claim records will be created as part of the genesis state. However for test net purposed and for the ability to allow UI developers a really easy way to test, we should expose a message handler that allows anyone to create a claim.

We will delete this prior to mainnet and should probably add in some logic to check the environment or chain id (creating a ticket for that now :)

AC

1. implement a CreateClaim transaction type that allows anyone to create a new claim record for any user for Arkeo or Eth
2. Determine best mechanism to branch based on testnet / mainnet

Currently the proto buf definitons in the arkeo module dont all follow the same naming convention.

We should refactor all of the definitions to match the preferred lower_case formatting style for consistency.

AC

• all proto buf files in ./proto/arkeo/arkeo/* are formatted correctly
• tests and builds passing with updated names
• update any command line args, ts files, etc to match

Background

See airdrop spec here.

We need to create functionality to import a JSON file that contains all of the ClaimRecords for the airdrop for the genesis state.

We also want to give all addresses that are part of the airdrop some minimal amount of arkeo (as configured) to pay for gas of their first claim. We may be able to not have any gas fees for this, but dusting these accounts should be simple enough for a fallback mechanism. This would obviously only occur for Arkeo claims (not eth based claims)

Some examples of genesis account creation functionality from osmosis -

https://github.com/osmosis-labs/osmosis/blob/v4.2.0/cmd/osmosisd/cmd/airdrop.go
https://github.com/osmosis-labs/osmosis/blob/v4.2.0/cmd/osmosisd/cmd/genaccounts.go

AC

1. A sample JSON object of ClaimRecords can be imported on genesis
2. Basic test to validate the correct import has occurred, native arkeo accounts have dust amounts for gas, and that claim records exist for Arkeo and Eth.

At the moment in sentinel, when query contract from arkeo daemon , it is still using http call.
GRPC is recommended , as it is more efficient.

If state can be committed to a block even when the transaction errors, we need to consider how to error handle and revert to original state. @johnnyluo suggests this is the case, while the comsos docs suggest otherwise afaict

Background

Currently the codebase is using a mixture of ints and uints primitives for most of the accounting in both the claim and arkeo module. This could be potentially problematic without implementing some sort of overflow protection or atleast reasoning about all the possible outcomes of integers wrapping.

The cosmos sdk offers an int that has overflow protection built in and is probably the right choice to fix this problem.

AC

1. identify all areas where primitive integer math is being used (addition, subtraction, multiplicatiion) and replace with safe math that will panic on an overflow

Background

Currently we allow a provider to un-bond at any time regardless of the number of open contracts that may exist. Subscription type contracts can cancel at any time, so this is fine for them, but a PayAsYouGo contract is not able to cancelled until it runs it course.

We should change this in the case that the provider has unbonded.

AC

1. Allow a PayAsYouGo contract to be closed in the provider has unbonded
2. Add tests to ensure this scenario works and doesn't work still when the provider is bonded.

Background

Currently a pay as you go user receives a refund for any remaining deposit on the block that their contract closes. This is problematic for a provider who will have the potential to not realize all of their due income by calling ClaimContractIncome before the refund is issued.

A simple mitigation for this is to have add to the contract states of PayAsYouGo contracts a settlement period in which no additional calls can be made against the contract, but the deposit isn't refunded. This window allows for the provider to claim the full amount prior to the contract automatically issuing a refund.

This adds some state complexity as currently a single user can only have one open contract with a provider, so we need to handle a contract in the settlement state differently than an open contract.

Acceptance criteria

1. Add a new contract state for the settlement period that is configurable to a number of blocks
2. Update sentinel to not allow additional calls against a PayAsYouGo contract that is in a settlement state
3. Ensure that a user who has a contract in settlement state can still open a new contract.
4. add all needed tests to ensure that refunds occur only after the settlement period has expired

Eventually we will build out some additional scripts for a provider to run that will automatically call claim income on their contracts during the settlement period.

Background

Currently internal testers have been working against this document
https://www.notion.so/Round-1-Stagenet-Testing-Docs-8f459efa5281429780044efb57130598
There are a variety of different issues that have been encountered in getting through the entire test suite.

For phase 1 to be considered done we should meet the below AC

Acceptance Criteria

• above notion doc is updated to include the needed steps from a clean environment with no dependencies installed to completion of all tests steps. We may want to document some of setup steps elsewhere, perhaps in the README.md since they may be used for other development purposes.
• All bug fixes needed to complete the outlined steps on the Arkeo chain itself have been completed.
• Verified by one other developer that they can take a fresh ec2 instance from nothing to completing all steps.

Other notes

We are spinning up a new seed node to test against in the cloud. @asamere can fill in details to access it once complete.

Background

During phase 1 testing it was discovered that if a contract is opened and then closed or exhausted, that it blocks the users ability to use the free tier from the same provider.

The desired behavior should be that it defaults back to the free tier (if it exists) instead of failing

AC

1. identify states in which a contract cannot serve requests, but the free tier could
2. Add tests for these states and ensure we revert back to using the free tier
3. We should check both for payasyougo and subscription based contracts to ensure we cover all possible permutations.

We currently do not support websockets as contract endpoints in sentinel. We need to discuss an approach for accounting/billing websocket usage.

this will function in an identical manner to ClaimEth but do so with a thorchain address and signature.

Background

Providers (or anyone) needs to ClaimIncome on PayAsYou go contracts as they are settled. We should provide an easy script that can be ran by a chronjob for providers to use to claim.

not required for launch, but nice to have.

Background

Current UserContractSets continually grow in size.

Ideally these sets only contain active contracts per user. Once a contract is completely dead, we can remove it from this set.

AC

1. prune UserContractSets as contracts are completely expired
2. Create tests to confirm this occurs only when a contract is completely dead and no other time.

1. claimEth passes with no native arkeo claim and delivers correct tokens for claim action
2. claimEth passes with a native arkeo claim and values are merged
3. claimEth fails when called a second time from same address
4. claimEth fails with invalid signature
5. claimEth fails when no claim record present
6. claimForAction passes on arkeo for native events (voting, delegating)
7. claimArkeo passes and delivers correct amount of tokens
8. claimArkeo fails when no claimRecord exists
9. event hooks are functional and emit tokens for airdrop claims for voting or delegating
10. event hooks dont create any issues when voter or delagator has no aidrop

sdkerrors.Wrapf is deprecated , the library has been moved to a different module,
it is recommended to use "cosmossdk.io/errors" directly

Currently, sentinel is refusing to proxy requests made under valid pay-as-you-go contracts, with a valid arkAuth signature. This may have something to do with creating a pay-as-you-go contract with a client key that previously had a subscription contract with the same provider.

I believe this is the sole blocker to completing stage1 testing.

Background

Currently, we have a few different ways we represent a "chain" in Arkeo and the proto bufs that seem to have the potential to present problems.

First, chain is probably a misnomer, since these are various api services that could be anything from a btc node, to unchained, to a completely unrelated market data api (as an example). So we should refactor to a more clear naming convention.

Second, any chain that doesn't fit into the defined chain types, looks like it will end up as unknown for indexing purposes which could lead to KV store collisions as its part of the key.

AC

1. refactor chain to a better naming convention
2. allow any string so chain is a provider specific construct
3. ensure all tests are working.

The downside of this approach is that providers may name things differently, for example btc-mainnet could be called btc-fullnode by 2 providers serving the same data.