Full Contract Functionality Coverage in CI Coverage #24
Comments
Contract functionality listas developer i must be able to:
as BLD stakers, we must be able to:
As eligible client i must be able to:
As a token holder i must be able to:
As market participant:
@dckc let me know your thoughts. also, i'm not sure if the spec changes this (if at all). the details in there seem to be more in the weeds than needed here |
|
looks about right. some editorial suggestions:
looks incomplete. I suggest: "do cosmos transfer (which demonstrates that IBC should work)"
I suggest: prevent ineligible parties from claiming tokens |
specify recipients - diagramp.s. see Sep 4 update. Here's a suggested diagram: sequenceDiagram
title: specify recipients by publishing bundle with merkleRoot
actor Dev
box lightgreen Dev Box
participant dropBuilder
end
box wheat Borwser
participant Keplr_D
participant cosgov
end
Dev ->> dropBuilder: yarn codegen
note right of dropBuilder: fetch addresses<br>compute merkleRoot
dropBuilder -->> Dev: contractBundle(merkleRoot)
participant CometBFT
box aqua JSVM
participant vstorage
participant bundleStore
participant BLDgov
end
Dev ->> cosgov: contractBundle(merkleRoot)
Dev ->> cosgov: publish
cosgov -->> Keplr_D: sign(bundleTx)
Keplr_D -->> Dev: ok to sign?
Dev -->> Keplr_D: ok. sign.
Keplr_D -->> bundleStore: contractBundle(merkleRoot)
bundleStore -->> vstorage: ok
cosgov -->> vstorage: query bundles
vstorage -->> cosgov: bundleHash,ok
note left of BLDgov: no BLDgov action yet
|
agoric-basics has tests for a bunch of contract functionality, but it does take some short-cuts. At the time, Crabble Protocol was ahead of the pack:
The test code there is lower level than |
|
the repository is private. any input on how to gain access? @dckc |
|
Ah. I didn't realize it's private. It's not a good reference, then. @Jovonni I hope you can help here.
|
|
This was not among a What's left for MN2 checklist? list as of Oct 9, which suggests that it should be closed. Comparing the most recent ci run I can see, ci run for #105 of Sep 10 against the May 3 checklist above, it looks like lots of stuff is covered, but testing cosmos-level transfer of claimed tokens depends on addressing #110 The comparison is a little challenging. It would help if there were tests with names that clearly corresponded. So I might be off in some cases here... Cross-checkas developer i must be able to:
as BLD stakers, we must be able to:
As eligible client i must be able to:
As a token holder i must be able to:
As market participant:
Test EnumerationThe ci log shows In particular: |
i'll see that I get to more suitably named tests as soon as possible.
this change is in the repository. the tests that check this are in progress #111 |
|
I see a new v0.1.1 release 02f32be - did the full contract functionality get tested in ci for that release? #121 was fixed in #127 , but I see "1 check failed", namely all. Somebody asked about a detail of the contract; I expected to be able to confirm from ci logs, but I'm struggling. |
|
hey @dckc - thanks for bringing this to my attention! thank you for calling out #121 in particular, that is a really helpful bit of information. I was also asked about this particular aspect of the token. I can confirm that the tokens are transferable. This is not an aspect of the token I had any part in enabling, and thus, I'm unaware of where the property of "transferability" emerges within the stack. My suspicions lead me to believe it's a combination of ERTP, cosmic-swingset, SwingSet, and agoric-sdk/golang. I'm wrapping up my findings on xnet stress testing, and then setting out to fix any CI issues with #115. I can add this as an issue though if you would like to see a test for it? To be honest, I had never considered a situation in which a wallet receives tokens, only for the owner of the wallet to find out they cannot be transferred. I'm struggling to imagine any example scenarios of this, so any input on that front would be useful. While it doesn't seem to be as much of an issue within the Cosmos ecosystem, the greater Ethereum ecosystem is rife with airdrop scams in which bad actors prey on (and pray for) the receiver to interact with tokens in order to gain access to the wallet and siphon out any value. |
|
setting aside transferrability, is Full Contract Functionality Coverage in CI still in effect? Did full contract functionality get exercised in ci for v0.1.1 release 02f32be? |
|
Yes, the changes are in effect and the codebase is updated with all existing tests. Regarding CI for v0.1.1 release - no, full contract functionality was not exercised. Anticipates feedback about releases requiring full verification While I have experience with releases in enterprise environments (BoA, FRBNY), this is my first foray into GitHub-based open source releases. @Jovanni provided guidance in Dubai, leading to my first releases (v0.1.0 and v0.1.1). I approached these as learning opportunities for Agoric dapp releases while simultaneously handling performance testing work. I acknowledge this led to CI verification falling into a blind spot, so thank you for surfacing this. I've since resolved any CI issues in #133. As a bonus perk, your response brought another item to surface which I fixed in #137. |
|
I see #133 restored CI to working order and #137 expands coverage. Cool.
It follows from
that releases would go through CI. The norm is to use github branch protection so that the robots prevent landing any code that doesn't pass ci. It would be nice to have something in v0.1.1 release notes to say that there's no ci log corresponding to exactly this version, but there is a ci log corresponding to something with no substantive differences. The title of #133 , "Restore Test Suite Integrity and Lint Compliance", suggests it might serve this purpose, but on closer look I see it's 27K LOC, so it's not clear at a glance that it has no substantive changes. |
Thanks for this. I'll add an issue and see that the repo is protected, finally putting an end this cat-and-mouse game 🤦.
Noted. I'll update the release to say as much.
I'm just taking a look at this PR and quickly suss out the main culprit - test account files. since I am testing the claiming process and required to have a significant number of account on hand so that I can add them to my key ring and then use them as necessary. Prior versions of the code base contained ~30 accounts
I also had to re-commit code that was previously in the repostiory, but had got lost along the way...
if only there were strict github branch rules in place that eliminated this from happenning... 😄 |
Task: Ensure that your contract functionality is covered with automated testing and new changes are covered by Continuous Integration.
Purpose: To ensure previously reviewed contract continues functioning properly after any changes are made
The text was updated successfully, but these errors were encountered: