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

Cryptex Milestone #3 #1031

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Cryptex Milestone #3 #1031

merged 2 commits into from
Nov 2, 2023

Conversation

driemworks
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, an invoice must be submitted and the payment will be transferred to the BTC/ETH/fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1660

@dsm-w3f dsm-w3f self-assigned this Oct 23, 2023
@dsm-w3f
Copy link
Contributor

dsm-w3f commented Oct 23, 2023

@driemworks thank you for the milestone delivery. Please see the evaluation document and provide proper answers and fixes. After that, let me know when I can continue this evaluation.

@carloskiron
Copy link

@driemworks thank you for the milestone delivery. Please see the evaluation document and provide proper answers and fixes. After that, let me know when I can continue this evaluation.

Thanks @dsm-w3f in relation to the ui issue, could you share with us the .env file you put in place? this will help us find the roort cause of your issue. Thanks!

@driemworks
Copy link
Contributor Author

The issue with the contract tests is resolved. I've sent tokens to your address as well.

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Oct 24, 2023

@carloskiron .env.local, the first variable is the proxy address

NEXT_PUBLIC_CONTRACT_ADDRESS="5Egh67rszZYc6WZxQLtnQzZxk3Tpwqq5vxcXGEPgSwU4uCAu"
NEXT_PUBLIC_NODE_DETAIL="ws://127.0.0.1:9944"

@driemworks
Copy link
Contributor Author

@dsm-w3f the .env configuration looks as expected. We resolved the error you were seeing logged in etf-auction-ui Specifically, next.js attempts to read the etf-sdk wasm when it imports etf.js (etf-sdk is a transitive dependency), however next.js expects that our assets are hosted in a CDN, so it tries to fetch the wasm from /_next, which results in an invalid URL (again, since we have not configured a CDN). However, the etf-auction-ui doesn't actually need to fetch that wasm since it's packaged with etf.js, so although it logged an error there is no impact to functionality. It should also be noted that when built as a static site (i.e. our 'official' build hosted at https://auction.idealabs.network), the error does not occur. Please proceed with your evaluation.

@carloskiron
Copy link

Btw @dsm-w3f something we forgot to mention in our previous reply is that the changes we did to solve the issues in the UI require an npm install again. It's better if you delete the nodes_modules folder and start over.

@dsm-w3f
Copy link
Contributor

dsm-w3f commented Oct 25, 2023

@carloskiron and @driemworks thank you for the improvements and fixes. We made some progress in the evaluation but there are still some points to fix and adjust. Please see the evaluation document for the updates. Let me know when I can continue this evaluation.

@driemworks
Copy link
Contributor Author

I can't be sure but it's likely the error you saw when running from your local was due to an NFT id collision (we don't validate anything before attempting to mint currently, so that's a possibility).

I've addressed all the other issues you were seeing, specifically the doc typo is fixed here and the 'min bid' display has been corrected. As for the error messages:

  • the RPC-CORE error was a result of not running an archive node. After updating the pruning config, we no longer see the error logged.
  • The CONTRACTS error is a little more complicated. The error was coming from here in api-contracts. It expects a contract to have at least one event defined. However, our tlock proxy contract emits no events, instead we are handling results returned from calls to the contract functions (while vickrey_auction and erc721 both do emit events). After adding an event, the error is removed. related open issue

the latest deployed contract id's are:
proxy: 5FxQpbpTqNgmdCTxvo21UKooMEWYceFAYtK38TA12EuJjTh6
erc721: 5CATVMp5sdBnJFqMBT8xno2cNf8k9V2swnCNHNNtn1jBpJrf

@dsm-w3f dsm-w3f removed their assignment Oct 27, 2023
@driemworks
Copy link
Contributor Author

driemworks commented Oct 27, 2023

@Noc2 I noticed this change to the team and I'm wondering how reviewer reassignment/re-review works.

Edit: + @keeganquigley (since we have similar timezones I believe)

@keeganquigley keeganquigley self-assigned this Oct 30, 2023
@keeganquigley
Copy link
Contributor

keeganquigley commented Oct 30, 2023

Thanks @driemworks yes there has been a recent personnel change. I can take over this delivery evaluation but please note that it may be a couple days before I can resume it since we now have a backlog. Shouldn't be longer than that since you are near the front of the queue :)

@keeganquigley
Copy link
Contributor

@driemworks Could you send some test tokens to this address? 5H1SgrToMp1RYhbUjzQmVxpRGRvo3s2j4xKUjCWJ9GQAVREM thanks!

@driemworks
Copy link
Contributor Author

@keeganquigley sent!

@keeganquigley
Copy link
Contributor

keeganquigley commented Nov 2, 2023

Thanks for your patience while I got up to speed. So far, all of the previous errors have been resolved, and I can get the node running. I can also use the UI to create an auction and place bids.

However, one test still fails when running cargo +nightly test from the root of substrate etf branch:

test tests::beefy_reports_equivocations ... FAILED
test tests::gossipped_finality_proofs ... ok
test tests::lagging_validators ... ok
test tests::should_initialize_voter_at_latest_finalized ... ok
test worker::tests::should_vote_target ... ok
test worker::tests::test_oracle_accepted_interval ... ok
test worker::tests::vote_on_mandatory_block ... ok
test worker::tests::vote_on_min_block_delta ... ok
test worker::tests::vote_on_power_of_two ... ok
test worker::tests::vote_on_target_block ... ok
test tests::correct_beefy_payload ... ok
test worker::tests::should_finalize_correctly ... ok
test worker::tests::should_init_session ... ok
Nov 02 00:04:27.811 ERROR gossip: Got message from unregistered peer who=12D3KooWT1x2Bjb2Z4E3ZJJSp1bh9ehWpkn6yauPbxT1i7LeDcLN protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
Nov 02 00:04:27.812 ERROR gossip: Got message from unregistered peer who=12D3KooWCYjWzrkzTg1samBXhesJko9hWmJq5J2s6p3J9zm45xtn protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
Nov 02 00:04:27.812 ERROR gossip: Got message from unregistered peer who=12D3KooWQMMxRFZbeRr5DkEEkgDyQsrnydUbKeaMeKq8duuQ4LnV protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
Nov 02 00:04:27.815 ERROR gossip: Got message from unregistered peer who=12D3KooWT1x2Bjb2Z4E3ZJJSp1bh9ehWpkn6yauPbxT1i7LeDcLN protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
Nov 02 00:04:27.816 ERROR gossip: Got message from unregistered peer who=12D3KooWCYjWzrkzTg1samBXhesJko9hWmJq5J2s6p3J9zm45xtn protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
Nov 02 00:04:27.816 ERROR gossip: Got message from unregistered peer who=12D3KooWQMMxRFZbeRr5DkEEkgDyQsrnydUbKeaMeKq8duuQ4LnV protocol=/0000000000000000000000000000000000000000000000000000000000000000/beefy/2
test tests::should_initialize_voter_when_last_final_is_session_boundary ... ok
test worker::tests::keystore_vs_validator_set ... ok
test worker::tests::should_not_report_bad_old_or_self_equivocations ... ok
test tests::on_demand_beefy_justification_sync ... ok

failures:

---- tests::beefy_reports_equivocations stdout ----
thread 'tests::beefy_reports_equivocations' panicked at client/consensus/beefy/src/tests.rs:1274:5:
assertion `left == right` failed
  left: 0
 right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::beefy_reports_equivocations

test result: FAILED. 47 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 18.74s

error: test failed, to rerun pass `-p sc-consensus-beefy --lib`

There are also a number of warnings when running cargo clippy. Can you check to see if these can be easily fixed?

@driemworks
Copy link
Contributor Author

The cargo clippy warnings are as a result of using a somewhat outdated fork of substrate (~4 months old or so, from when we started work on the grant). After the grant is completed we're going to separate our consensus modules into their own repo and update to the new dependencies in the polkadot-sdk monorepo, which I suspect would resolve the clippy warnings. I have verified that it does not call out anything in the sc-consensus-aura, in our runtime config, or in our new pallet.

About the test failure, I'm not sure why that would happen. Our changes wouldn't have had an impact on those tests. I executed it locally against the etf branch and it passed. Can you run the test by itself to verify? I know it's a tokio test, so perhaps an issue with the async execution.

image

@keeganquigley
Copy link
Contributor

keeganquigley commented Nov 2, 2023

Thanks @driemworks here is my final evaluation. I'm not too worried about it since it is just dealing with node gossip, and not part of this milestone scope. It could be something on my end. Everything is working as expected, and I could use the app both locally and on the testnet version. I'm happy to pass the milestone. Congrats on finishing the grant! Nice work!

@keeganquigley keeganquigley merged commit b60c6f4 into w3f:master Nov 2, 2023
6 checks passed
Copy link

github-actions bot commented Nov 2, 2023

We noticed that this is the last milestone of your project. Congratulations on completing your grant! 🎊

So, where to from here? First of all, you should join our Grants Community chat, if you haven't already, so we can stay in touch.
If you are looking for continuative support for your project, there are quite a few options. The main goal of the W3F grants program is to support research as well as early-stage technical projects. If your project still falls under one of those categories, you might want to apply for a follow-up grant. However, depending on your goals and project status, there are other support programs in our ecosystem that might be better suited as the next step. For example, projects with a Business Case/Token should look into the Substrate Builders Program or VC Funding and Common Good projects have a good chance of receiving Treasury Funding. If you are looking for guidance, the team at https://substrate.io/ecosystem/square-one/ has compiled a list of ecosystem support sources and are happy to help you navigate it.

For a more comprehensive list, see our Alternative Funding page. Let us know if you have any questions regarding the above. We are more than happy to point you to additional resources and help you determine the best course of action.
Lastly, we hope your W3F grant was a success and we want to thank you for being part of the journey!

Copy link

github-actions bot commented Nov 2, 2023

🪙 Please fill out the invoice form in order to initiate the payment process. Thank you!

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