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

Reference states prototype (for DRB) #2889

Closed
wants to merge 11 commits into
base: release-V3
from

Conversation

Projects
None yet
4 participants
@roger3cev
Copy link
Member

roger3cev commented Mar 28, 2018

High level design here: corda/enterprise#632

Need to fix failing tests.

roger3cev added some commits Mar 7, 2018

* Added ReferenceData interface.
* Added new transaction component group.
* Updated transaction types to include an unspendable states list.
* Added a test to show it working with some notes.
* Updated name of new component group to unspendableInputs.
* Removed ReferenceState interface as it isn't necessary if it's OK for any ContractState to be used as an unspendable input.
* Deprecated TransactionBuilder.withItems.
* Added more tests and cleaned up the tests.
* Modified the TestDSL to take into account use of old unspendable states.
* Updated SignedTransaction with convenience method for getting unspe…
…ndable states.

* Updated service side of abstract notary flow.
* Updated the some of the required types for the notary services to include unspendable states.
* Updated the notary commit logic to check for conflicts with unspendable states but not commit them.
Merge remote-tracking branch 'origin/rog-unspendable-inputs' into rel…
…ease-V3

# Conflicts:
#	core/src/main/kotlin/net/corda/core/flows/NotaryFlow.kt
#	core/src/main/kotlin/net/corda/core/node/services/NotaryService.kt
#	core/src/main/kotlin/net/corda/core/node/services/UniquenessProvider.kt
#	core/src/main/kotlin/net/corda/core/transactions/ContractUpgradeTransactions.kt
#	core/src/main/kotlin/net/corda/core/transactions/NotaryChangeTransactions.kt
#	node/src/main/kotlin/net/corda/node/services/transactions/PersistentUniquenessProvider.kt
* Renamed upsendableStates to reference states by recommendation from…
… Mike.

* Bug fix: Removed reference state contracts from the list of contracts to run verify on in LedgerTransaction.
* Updated to Corda V3.
* Minor unit test changes.
* Made the API additions backwards compatible.
* Added @deprecated withItems to the api-current file.
* TODO: Docs.

@roger3cev roger3cev requested review from mikehearn , gendal , mnesbit and rick-r3 Mar 28, 2018

@fenryka
Copy link
Contributor

fenryka left a comment

I think this isn't wire stable because of the changes to the ENUMS

@@ -4,7 +4,7 @@ buildscript {
file("$projectDir/constants.properties").withInputStream { constants.load(it) }

// Our version: bump this on release.
ext.corda_release_version = "corda-3.1-snapshot"
ext.corda_release_version = "corda-3.1-roger"

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

Probably don't mean to check this in as-is

This comment has been minimized.

@roger3cev

roger3cev Mar 29, 2018

Author Member

"corda-3.1-roger" is my fork of Corda :)

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

Yes, but it shouldn't be in the PR.

@@ -11,5 +11,6 @@ enum class ComponentGroupEnum {
ATTACHMENTS_GROUP, // ordinal = 3.
NOTARY_GROUP, // ordinal = 4.
TIMEWINDOW_GROUP, // ordinal = 5.
SIGNERS_GROUP // ordinal = 6.
SIGNERS_GROUP, // ordinal = 6.

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

This breaks wire stability

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

Look at adding a transform mapping for the new entries and we'd need to test this 3.0 -> 3.1

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

and 3.1 -> 3.0

This comment has been minimized.

@rick-r3

rick-r3 Apr 4, 2018

Contributor

The enum isn't sent over the wire. It's sent as an Int.

This comment was marked as resolved.

@mikehearn

mikehearn Apr 24, 2018

Contributor

Yes, I think this enum is 'special' and does not need to use the serialiser enum evolution feature.

@@ -100,7 +104,7 @@ data class ContractUpgradeWireTransaction(
}

enum class Component {
INPUTS, NOTARY, LEGACY_ATTACHMENT, UPGRADED_CONTRACT, UPGRADED_ATTACHMENT
INPUTS, NOTARY, LEGACY_ATTACHMENT, UPGRADED_CONTRACT, UPGRADED_ATTACHMENT, REFERENCES

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

same issue as above

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

although if this isn't ever serialised it's not a problem

@@ -40,7 +40,9 @@ data class LedgerTransaction @JvmOverloads constructor(
override val notary: Party?,
val timeWindow: TimeWindow?,
val privacySalt: PrivacySalt,
private val networkParameters: NetworkParameters? = null
private val networkParameters: NetworkParameters? = null,
/** The resolved reference states. */

This comment has been minimized.

@fenryka

fenryka Mar 28, 2018

Contributor

wouldn't this be better as an property jdoc

This comment has been minimized.

@roger3cev

roger3cev Mar 29, 2018

Author Member

True that. Will add. Tbh I din't think I have actually added any jdocs yet. I'll stick it as a TODO.

@roger3cev

This comment has been minimized.

Copy link
Member Author

roger3cev commented Mar 29, 2018

Thanks for the comments @fenryka - could I catch up with you to discuss the enum changes required? Cheers

@mikehearn
Copy link
Contributor

mikehearn left a comment

Looks good so far. We will probably want a new flow as discussed to handle the suspend/wait/retry loop when a reference state conflicts. It can be done in this PR or a different one.

@@ -4,7 +4,7 @@ buildscript {
file("$projectDir/constants.properties").withInputStream { constants.load(it) }

// Our version: bump this on release.
ext.corda_release_version = "corda-3.1-snapshot"
ext.corda_release_version = "corda-3.1-roger"

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

Yes, but it shouldn't be in the PR.

@@ -11,5 +11,6 @@ enum class ComponentGroupEnum {
ATTACHMENTS_GROUP, // ordinal = 3.
NOTARY_GROUP, // ordinal = 4.
TIMEWINDOW_GROUP, // ordinal = 5.
SIGNERS_GROUP // ordinal = 6.
SIGNERS_GROUP, // ordinal = 6.

This comment was marked as resolved.

@mikehearn

mikehearn Apr 24, 2018

Contributor

Yes, I think this enum is 'special' and does not need to use the serialiser enum evolution feature.


/**
* Topologically sorts the given transactions such that dependencies are listed before dependers. */
* Request unspendable inputs as well as regular inputs.

This comment was marked as resolved.

@mikehearn

mikehearn Apr 24, 2018

Contributor

A better description might be "Returns the hashes of all direct dependencies of this transaction"

fun commit(states: List<StateRef>, txId: SecureHash, callerIdentity: Party)

fun commit(states: List<StateRef>, txId: SecureHash, callerIdentity: Party, references: List<StateRef>)

This comment was marked as resolved.

@mikehearn

mikehearn Apr 24, 2018

Contributor

KDoc on this one too please.

* [clazz] must be an extension of [ContractState].
* @return the possibly empty list of inputs [StateAndRef] matching the clazz restriction.
*/
fun <T : ContractState> refRefsOfType(clazz: Class<T>): List<StateAndRef<T>> {

This comment was marked as resolved.

@mikehearn

mikehearn Apr 24, 2018

Contributor

This could probably have a better name.

val notary = stateAndRef.state.notary
require(notary == this.notary) { "Input state requires notary \"$notary\" which does not match the transaction notary \"${this.notary}\"." }
}

open fun addReferenceState(stateAndRef: StateAndRef<ContractState>): TransactionBuilder {

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

KDoc

return this
}

open fun addInputState(stateAndRef: StateAndRef<*>): TransactionBuilder {

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

KDoc (i know it was missing already)

@@ -208,6 +224,7 @@ open class TransactionBuilder(
}

// Accessors that yield immutable snapshots.
fun referenceStates(): List<StateRef> = ArrayList(references)

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

kdocs

timeWindow: TimeWindow?,
privacySalt: PrivacySalt = PrivacySalt()
) : this(createComponentGroups(inputs, outputs, commands, attachments, notary, timeWindow), privacySalt)
@JvmOverloads constructor(inputs: List<StateRef>,

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

Do we need to update this if it's deprecated? It's OK to not update old APIs with new features.

/**
* Looks up the output label and adds the found state as an input.
* @param stateLabel The label of the output state specified when calling [TransactionDSLInterpreter.output] and friends.
*/
fun input(stateLabel: String) = input(retrieveOutputStateAndRef(ContractState::class.java, stateLabel).ref)

/**
* Creates an [LedgerDSLInterpreter._unverifiedTransaction] with a single output state and adds it's reference as an
* Creates an [LedgerDSLInterpreter._unverifiedTransaction] with a single input state and adds it's reference as an

This comment has been minimized.

@mikehearn

mikehearn Apr 24, 2018

Contributor

That change isn't correct, is it?

@roger3cev roger3cev closed this May 2, 2018

@roger3cev roger3cev deleted the rog-reference-states branch May 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.