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

Implementing first pass of veAllocate schema, handlers, and test cove… #490

Merged
merged 83 commits into from
Sep 5, 2022

Conversation

idiom-bytes
Copy link
Member

@idiom-bytes idiom-bytes commented Jul 18, 2022

PR is WIP (not ready for merging)

  • Tracer bullet for veAllocate
  • Submitting this so we can track work + iterate towards a working subgraph

DoD:

[x] - Designed gql schema
[x] Implemented event handlers
[x] Implemented getters & basic functionality
[ ] Implement tx,block,ts to create & update events
[ ] Implement & validate tests
[ ] Sign off on schema and handler design/implementation

Identified issues outside current design scope:

See veAllocate.test.ts
[ ] veAllocate objects & interfaces inside of oceanprotocol/lib
[ ] implementing veAllocate + other functionality inside of contracts repo => deploy_pool_template.js. We need to add vyper dependencies + vyper compile steps into deploy_docker.sh and any other places required to deploy contracts via barge or other CI/CD workflows.

@idiom-bytes idiom-bytes requested a review from mihaisc July 18, 2022 20:30
@alexcos20
Copy link
Member

alexcos20 commented Jul 22, 2022

Ocean-contracts v1.1.0 was released, so make sure your local barge is using that by doing:

cd barge
export CONTRACT_VERSIONS=v1.1.0
./start_ocean.sh ......

(see https://github.com/oceanprotocol/ocean-subgraph/blob/feature/veAllcocate/.github/workflows/tests.yml#L55)

@mihaisc
Copy link
Contributor

mihaisc commented Jul 25, 2022

Please use pascal case in schema.graphql , for example veAllocation -> VeAllocation

@idiom-bytes
Copy link
Member Author

idiom-bytes commented Jul 29, 2022

A first pass for this has been completed.

Next Steps for Feature/Epic:

subgraph - veAllocate - Add tx/block/timestamps to subgraph entities
dfpy test_query + data flows - Finishing df-py test_query + starting to query subgraph
dfweb - contract calls - Implement contract calls (create lock, has lock, extend lock, get voting power, etc..)

Review:

It would be good to get feedback on schema/structure/parameter names

How to test

  • install dfpy
  • use the dfpy tool command
dftool veSetAllocation CHAINID amount exchangeId
dftool veSetAllocation 8996 100 test1

committed to ocean-subgraph

  • properly handles setAllocation + removeAllocation
  • rolling totals for user, exchangeId, and each combination works
  • you can also see the Set/Remove events
  • verified that tests that do not expect provider/aquarius are working
  • verified that the data is looking correct

Recommendation - Harden veAllocation.setAllocation()

Add a cost to allocate. Users can currently DDOS the subgraph by injecting a bunch of garbage allocates. Make the fee from this go back to veOcean/farmers.

Recommendation - User-Friendly AllocationId:

The only thing I don't like about this, is that we need to do a bunch of offchain ingestion/calculations/mappings to understand which exchangeId + chainid the user is interacting with. It's not obvious by just looking at the data.

Consideration: Perhaps it might be a good idea if veAllocate keeps track of exchangeId + chainId, and can return that in the events. This way, we could use that information (exchangeId + chainId) in the subgraph. This would make the mapping a lot easier.

Screenshot from 2022-07-28 16-56-18

Queries

{
  veAllocations(first: 1000) {
    id
    allocationUser {
      id
    }
    allocationId{
      id
    }
    allocatedTotal
  }
}

{
  veAllocateUsers(first: 1000) {
    id
    allocatedTotal
  }
}

{
  veAllocateIds(first: 1000) {
    id
    allocatedTotal
  }
}

{
  veAllocationUpdates(first: 1000) {
    id
    veAllocation {
      id
      allocationUser{
        id
        allocatedTotal
      }
      allocationId {
        id
        allocatedTotal
      }
    }
    type
    allocatedTotal
  }
}

@trizin trizin requested review from mihaisc and alexcos20 August 11, 2022 10:53
@trizin
Copy link
Contributor

trizin commented Aug 11, 2022

Tests fail because veAllocate contract is out of date.

trizin and others added 2 commits August 17, 2022 08:49
…he user current allocation, maybe we need to create a feature to more easily let them know whether they are fully allocated, or not..
@trizin trizin force-pushed the feature/veAllcocate branch from 36e130a to eccd3e5 Compare August 18, 2022 09:46
@idiom-bytes
Copy link
Member Author

idiom-bytes commented Aug 18, 2022

Fixed conflict.

New version of ocean-contract module has to be deployed so CI/CD can pass.
oceanprotocol/contracts#690

for (let line = 0; line < lines.length; line++) {
subgraph += ' ' + lines[line] + '\n'
}
}
Copy link
Member Author

@idiom-bytes idiom-bytes Sep 2, 2022

Choose a reason for hiding this comment

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

Thanks Alex, this "looks good" although crazy to see the need to fix indentation from the build.

Merge is still blocked awaiting for you or Mihai's review.

Copy link
Member

Choose a reason for hiding this comment

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

My VS Code will "fix" indentation for every save on subgraph_ve.yaml, making impossible to join them :)

Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

I will merge it but:

  • DF contracts are not handled
  • there are no tests

@alexcos20 alexcos20 merged commit a10c624 into main Sep 5, 2022
@alexcos20 alexcos20 deleted the feature/veAllcocate branch September 5, 2022 12:07
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.

4 participants