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

Fix: atlas attachment serving + broken test #2390

Merged
merged 3 commits into from
Jan 30, 2021
Merged

Fix: atlas attachment serving + broken test #2390

merged 3 commits into from
Jan 30, 2021

Conversation

lgalabru
Copy link
Contributor

After investigation, it turns out the atlas test was broken for a good reason.

  • Attachment serving broke with a recent refactoring
  • The BNS pricing table changed and the value provided was too low, as a consequence the namespace-preorder contract call was failing.

@lgalabru lgalabru self-assigned this Jan 29, 2021
@@ -387,7 +387,7 @@ impl AtlasDB {
content_hash: &Hash160,
) -> Result<Option<Attachment>, db_error> {
let hex_content_hash = to_hex(&content_hash.0[..]);
let qry = "SELECT content, hash FROM attachments WHERE hash = ?1 AND was_instantiated = 0"
let qry = "SELECT content, hash FROM attachments WHERE hash = ?1 AND was_instantiated = 1"
Copy link
Member

Choose a reason for hiding this comment

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

There haven't been any attachments propagated to the mainnet network yet, right? i.e. We're certain this change isn't going to break anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The BNS endpoints + registrar not being live at this point, the only interactions that happened with the contract were internal tests (https://explorer.stacks.co/txid/SP000000000000000000002Q6VF78.bns?chain=mainnet)

Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM; just had one question I need some clarity on first.

@lgalabru lgalabru requested a review from kantai January 29, 2021 20:27
@lgalabru lgalabru changed the base branch from master to develop January 29, 2021 20:27
@@ -16,9 +16,6 @@ jobs:
DOCKER_BUILDKIT: 1
run: docker build -f ./.github/actions/bitcoin-int-tests/Dockerfile.bitcoin-tests .
atlas-test:
# disable this job/test for now, since we haven't seen this pass
# on github actions in a while
if: ${{ false }}
Copy link
Member

Choose a reason for hiding this comment

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

🥳

@lgalabru lgalabru merged commit b9743ae into develop Jan 30, 2021
@lgalabru lgalabru deleted the fix/atlas branch January 30, 2021 03:34
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants