Skip to content
This repository has been archived by the owner on Jul 20, 2021. It is now read-only.

Add IOTA Tangle assessor. #8

Conversation

jbonafide623
Copy link
Contributor

Addresses: #4

Signed-off-by: Jason Bonafide [email protected]

@jbonafide623 jbonafide623 force-pushed the topic/iota-tangle-assessor branch from 9deeb52 to 8fa5485 Compare May 29, 2020 17:39
@michaelestrin michaelestrin changed the title Add IOTA Tangle assesor. Add IOTA Tangle assessor. Jun 2, 2020
Copy link
Contributor

@michaelestrin michaelestrin left a comment

Choose a reason for hiding this comment

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

Rebase.

Decouple IPFS. Basic assessor should simply validate the IOTA annotation from the local store points to valid content on the Tangle. Follow-on enhancement would be to provide a closure/interface to the assessor that would evaluate the content retrieved from the Tangle for correctness. The closure/interface implementation would live outside of the IOTA assessor. Do the basic assessor for this PR and we can work together on the enhancement as a separate issue/PR.

Implement factories and related support to ensure strongly-typed structures are unmarshaled.

Let's discuss if you have questions.

Signed-off-by: Jason Bonafide <[email protected]>
@jbonafide623 jbonafide623 force-pushed the topic/iota-tangle-assessor branch from 8fa5485 to 721a7a5 Compare June 4, 2020 21:20
@jbonafide623
Copy link
Contributor Author

@michaelestrin Update the PR with the mentioned changes:

  • Removed dependency of IPFS metadata from IOTA Assessor
  • Currently using simple approach of verifying IOTA Annotations to be assessed with IOTA Annotations which have been published to the Tangle

Copy link
Contributor

@michaelestrin michaelestrin left a comment

Choose a reason for hiding this comment

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

Update your commit comment to spell assessor properly.

Tests fail.

image

Missing factory implementation for IOTA assessor metadata.

Not a complete review.

@@ -40,9 +42,12 @@ import (
"github.com/project-alvarium/go-sdk/pkg/annotator/provenance"
"github.com/project-alvarium/go-sdk/pkg/annotator/publish"
publishMetadata "github.com/project-alvarium/go-sdk/pkg/annotator/publish/metadata"
publisherMetadataFactory "github.com/project-alvarium/go-sdk/pkg/annotator/publish/metadata/factory"
Copy link
Contributor

Choose a reason for hiding this comment

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

Alias should be publishMetadataFactory.

Comment on lines 276 to 281
filterFactory.New(
func(annotation *annotation.Instance) bool {
t, ok := annotation.Metadata.(*publishMetadata.Instance)
return ok && t.PublisherKind == iotaPublisherMetadata.Kind
},
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with passthroughFilter.

persistence,
iotaAssessor.New(
newClient(iotaURL),
publisherMetadataFactory.New([]metadataFactory.Contract{iotaMetadataFactory.New()}),
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with publisherMetadataFactory.NewDefault(). You'll likely extract this to a variable in a follow-on PR to reuse the resulting instance to assess IPFS publisher annotations.

@@ -57,3 +71,32 @@ func FactoryRandomFixedSizeBundle(size int) bundle.Bundle {
}
return txs
}

// FactoryAnnotationTransaction returns a Transaction comprised of a single annotation instance.
func FactoryAnnotationTransaction(t *testing.T, unique string, metadata metadata.Contract) transaction.Transaction {
Copy link
Contributor

Choose a reason for hiding this comment

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

t parameter isn't used; remove it.

Comment on lines 83 to 92
return &annotation.Instance{
Unique: unique,
Created: test.FactoryRandomString(),
CurrentIdentityKind: test.FactoryRandomString(),
CurrentIdentity: identityProvider.New(sha256.New()).Derive(test.FactoryRandomByteSlice()),
PreviousIdentityKind: test.FactoryRandomString(),
PreviousIdentity: nil,
MetadataKind: publishMetadata.Kind,
Metadata: metadata,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't initialize the struct directly; use annotation.New() instead. Using a factory function when one is available should always be preferred over direct assignment. The factory function's purpose is to provide a properly initialized receiver (something you're not guaranteed to get here when annotation.Instance evolves and this instance isn't updated).

),
),
},
client: stub.New([]transaction.Transactions{{}}, nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

{{}} -> {}

),
),
},
client: stub.New([]transaction.Transactions{{}}, nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

{{}} -> {}

),
),
},
client: stub.New([]transaction.Transactions{{}}, nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

{{}} -> {}

),
),
},
client: stub.New([]transaction.Transactions{{}}, err),
Copy link
Contributor

Choose a reason for hiding this comment

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

{{}} -> {}


"github.com/project-alvarium/go-sdk/pkg/test"

"github.com/magiconair/properties/assert"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong library. Use github.com/stretchr/testify/assert instead.

@michaelestrin
Copy link
Contributor

Note also your commit comment isn't auto-closing the corresponding issue.

image

Use a supported keyword in place of "Addresses" (see https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)

@michaelestrin
Copy link
Contributor

Closed without merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants