-
Notifications
You must be signed in to change notification settings - Fork 205
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
Support rollback nodes in PostCommitValidation #9501
Merged
Merged
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
efb4c0b
Support rollback nodes in PostCommitValidation
cocreature 5a270ee
Update ledger/participant-integration-api/src/main/scala/platform/sto…
cocreature 197c044
Update ledger/participant-integration-api/src/main/scala/platform/sto…
cocreature 8606a72
Update ledger/participant-integration-api/src/main/scala/platform/sto…
cocreature e31de28
Update ledger/participant-integration-api/src/main/scala/platform/sto…
cocreature 748b15f
Apply suggestions from code review
cocreature 75fdd3e
fmt
cocreature File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,29 +122,23 @@ private[dao] object PostCommitValidation { | |
transaction: CommittedTransaction, | ||
divulged: Set[ContractId], | ||
): Set[ContractId] = { | ||
val (createdInTransaction, referred) = | ||
transaction.fold((Set.empty[ContractId], Set.empty[ContractId])) { | ||
case ((created, ids), (_, c: Create)) => | ||
(created + c.coid, ids) | ||
case ((created, ids), (_, e: Exercise)) if !divulged(e.targetCoid) => | ||
(created, ids + e.targetCoid) | ||
case ((created, ids), (_, f: Fetch)) if !divulged(f.coid) => | ||
(created, ids + f.coid) | ||
case ((created, ids), (_, l: LookupByKey)) => | ||
(created, l.result.filterNot(divulged).fold(ids)(ids + _)) | ||
case ((created, ids), _) => (created, ids) | ||
} | ||
referred.diff(createdInTransaction) | ||
transaction.inputContracts.diff(divulged) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, I'd recommend to just inline this. |
||
} | ||
|
||
private def validateKeyUsages( | ||
transaction: CommittedTransaction | ||
)(implicit connection: Connection): Option[RejectionReason] = | ||
transaction | ||
.fold[Result](Right(State.empty(data))) { | ||
case (Right(state), (_, node)) => validateKeyUsages(node, state) | ||
case (rejection, _) => rejection | ||
} | ||
.foldInExecutionOrder[Result](Right(State.empty(data)))( | ||
exerciseBegin = (acc, _, exe) => { | ||
val newAcc = acc.flatMap(validateKeyUsages(exe, _)) | ||
(newAcc, true) | ||
}, | ||
exerciseEnd = (acc, _, _) => acc, | ||
rollbackBegin = (acc, _, _) => (acc.map(_.beginRollback()), true), | ||
rollbackEnd = (acc, _, _) => acc.map(_.endRollback()), | ||
leaf = (acc, _, leaf) => acc.flatMap(validateKeyUsages(leaf, _)), | ||
) | ||
.fold(Some(_), _ => None) | ||
|
||
private def validateKeyUsages( | ||
|
@@ -170,18 +164,49 @@ private[dao] object PostCommitValidation { | |
|
||
private type Result = Either[RejectionReason, State] | ||
|
||
/** The active ledger key state during validation. | ||
* After a rollback node, we restore the state at the | ||
* beginning of the rollback. | ||
* | ||
* @param contracts Active contracts created in | ||
* the current transaction that have a key indexed | ||
* by a hash of their key. | ||
* @param removed Hashes of contract keys that are known to | ||
* to be archived. Note that a later create with the same | ||
* key will remove the entry again. | ||
*/ | ||
private final case class ActiveState( | ||
contracts: Map[Hash, ContractId], | ||
removed: Set[Hash], | ||
) { | ||
def add(key: Key, id: ContractId): ActiveState = | ||
copy( | ||
contracts = contracts.updated(key.hash, id), | ||
removed = removed - key.hash, | ||
) | ||
|
||
def remove(key: Key): ActiveState = | ||
copy( | ||
contracts = contracts - key.hash, | ||
removed = removed + key.hash, | ||
) | ||
} | ||
|
||
/** Represents the state of an ongoing validation. | ||
* It must be carried over as the transaction is | ||
* validated one node at a time in pre-order | ||
* traversal for this to make sense. | ||
* | ||
* @param contracts All contracts created as part of the current transaction | ||
* @param removed Ensures indexed contracts are not referred to by key if they are removed in the current transaction | ||
* @param data Data about committed contracts for post-commit validation purposes | ||
* @param currentState The current active ledger state. | ||
* @param rollbackStack Stack of states at the beginning of rollback nodes so we can | ||
* restore the state at the end of the rollback. The most recent rollback | ||
* comes first. | ||
* @param data Data about committed contracts for post-commit validation purposes. | ||
* This is never changed durng the traversal of the transaction. | ||
*/ | ||
private final case class State( | ||
private val contracts: Map[Hash, ContractId], | ||
private val removed: Set[Hash], | ||
private val currentState: ActiveState, | ||
private val rollbackStack: List[ActiveState], | ||
private val data: PostCommitValidationData, | ||
) { | ||
|
||
|
@@ -204,29 +229,40 @@ private[dao] object PostCommitValidation { | |
else Left(MismatchingLookup(expectation, result)) | ||
} | ||
|
||
private def add(key: Key, id: ContractId): State = | ||
def beginRollback(): State = | ||
copy( | ||
contracts = contracts.updated(key.hash, id), | ||
removed = removed - key.hash, | ||
rollbackStack = currentState :: rollbackStack | ||
) | ||
|
||
def endRollback(): State = rollbackStack match { | ||
case Nil => | ||
throw new IllegalStateException("Internal error: rollback end but rollbackStack was empty") | ||
case head :: tail => | ||
copy( | ||
currentState = head, | ||
rollbackStack = tail, | ||
) | ||
} | ||
|
||
private def add(key: Key, id: ContractId): State = | ||
copy(currentState = currentState.add(key, id)) | ||
|
||
private def remove(key: Key): State = | ||
copy( | ||
contracts = contracts - key.hash, | ||
removed = removed + key.hash, | ||
currentState = currentState.remove(key) | ||
) | ||
|
||
private def lookup(key: Key)(implicit connection: Connection): Option[ContractId] = | ||
contracts.get(key.hash).orElse { | ||
if (removed(key.hash)) None | ||
currentState.contracts.get(key.hash).orElse { | ||
if (currentState.removed(key.hash)) None | ||
else data.lookupContractKeyGlobally(key) | ||
} | ||
|
||
} | ||
|
||
private object State { | ||
def empty(data: PostCommitValidationData): State = | ||
State(Map.empty, Set.empty, data) | ||
State(ActiveState(Map.empty, Set.empty), Nil, data) | ||
} | ||
|
||
private[events] val DuplicateKey: RejectionReason = | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a one-liner used only in one place I'd suggest to just inline it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered it but it seemed easier for review at least to only replace the implementation but leave it in a separate definition. Don’t feel strongly though so happy to inline it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call as a first step. 🙇🏻