Rename precondition.assert* to require*

[mitschabaude]

Why: The shared "assert" naming between constraints like x.assertEquals(0) and preconditions like this.state.assertEquals(Field(0)) is a constant source of confusion. We settled on using require() instead of assert() for preconditions to clearly mark it as a distinct concept.

How: Change all occurences of assert* in precondition methods, e.g.

• this.state.assertEquals() -> this.state.requireEquals()
• this.state.getAndAssertEquals() -> this.state.getAndRequireEquals() (is that too verbose?)
• this.account.balance.assertBetween() -> this.account.balance.requireBetween()

[LuffySama-Dev]

Hi @mitschabaude ,

I am get that you want occurrences of assert* to be changed and I would like to contribute.
So, if possible can you please help me understand in which files do you want occurrences changed I will try to understand them and start with it. More of like I want a starting point.

Thank you 😃

[mitschabaude]

Hey @Saurabhpatil-dev that would be very welcome!!

The API that we want to change is implemented in two files:

• src/lib/state.ts - onchain state
• src/lib/precondition.ts - other preconditions

It should be enough to change those two files, and then adapt all tests.

Important: Don't delete the old APIs, only deprecate them using the @deprecated doccomment

[LuffySama-Dev]

Awesome!!!

Let me try to get it done and up for review by tomorrow.😃

Thanks

[LuffySama-Dev]

Hi @mitschabaude ,

I have made changes in src/lib/state.ts file. #1263
Can you please help to review them ?

Also, I am not sure about the changes in src/lib/precondition.ts file. Can you please help me understand it little more ?

Thank You 😃

[mitschabaude]

@Saurabhpatil-dev sure, if you look at precondition.ts, you'll find the assert* API in a few places, and the goal would be to update all of them. This is what users will see when they do something like this.account.balance.getAndAssertEquals() in a smart contract.

Places to change:

• I think this is where we want the @deprecated tags! it defines the interface:
  

  o1js/src/lib/precondition.ts

  Lines 453 to 461 in 3261e8c

  	type PreconditionSubclassType<U> = {
  	get(): U;
  	getAndAssertEquals(): U;
  	assertEquals(value: U): void;
  	assertNothing(): void;
  	};
  	type PreconditionSubclassRangeType<U> = PreconditionSubclassType<U> & {
  	assertBetween(lower: U, upper: U): void;
  	};

• This and the others are just code that needs to change:
  

  o1js/src/lib/precondition.ts

  Lines 52 to 71 in 3261e8c

  	getAndAssertEquals() {
  	let slot = network.globalSlotSinceGenesis.getAndAssertEquals();
  	return globalSlotToTimestamp(slot);
  	},
  	assertEquals(value: UInt64) {
  	let { genesisTimestamp, slotTime } =
  	Mina.activeInstance.getNetworkConstants();
  	let slot = timestampToGlobalSlot(
  	value,
  	`Timestamp precondition unsatisfied: the timestamp can only equal numbers of the form ${genesisTimestamp} + k*${slotTime},\n` +
  	`i.e., the genesis timestamp plus an integer number of slots.`
  	);
  	return network.globalSlotSinceGenesis.assertEquals(slot);
  	},
  	assertBetween(lower: UInt64, upper: UInt64) {
  	let [slotLower, slotUpper] = timestampToGlobalSlotRange(lower, upper);
  	return network.globalSlotSinceGenesis.assertBetween(slotLower, slotUpper);
  	},
  	assertNothing() {
  	return network.globalSlotSinceGenesis.assertNothing();

• o1js/src/lib/precondition.ts

  Line 121 in 3261e8c

  assertBetween(lower: UInt32, upper: UInt32) {

• o1js/src/lib/precondition.ts

  Lines 235 to 259 in 3261e8c

  	getAndAssertEquals() {
  	let value = obj.get();
  	obj.assertEquals(value);
  	return value;
  	},
  	assertEquals(value: U) {
  	context.constrained.add(longKey);
  	let property = getPath(
  	accountUpdate.body.preconditions,
  	longKey
  	) as AnyCondition<U>;
  	if ('isSome' in property) {
  	property.isSome = Bool(true);
  	if ('lower' in property.value && 'upper' in property.value) {
  	property.value.lower = value;
  	property.value.upper = value;
  	} else {
  	property.value = value;
  	}
  	} else {
  	setPath(accountUpdate.body.preconditions, longKey, value);
  	}
  	},
  	assertNothing() {
  	context.constrained.add(longKey);

You can see your changes in TS by re-running npm run build, and then looking at src/lib/precondition.test.ts. You can run that test with ./jest src/lib/precondition.test.ts

[LuffySama-Dev]

Hi @mitschabaude ,

Okay, Will make a PR for this once the current PR is merged if it's fine ?

[LuffySama-Dev]

Hi @mitschabaude ,

While re-building the project locally I got the errors related to this PR you made yesterday #1260 .

Locally I am just reverting the changes locally for testing. Will not commit it.

[mitschabaude]

@Saurabhpatil-dev you need to update the submodule, git submodule update --init

[LuffySama-Dev]

Hey @mitschabaude ,

Thanks it worked.

The Precondition API's are also renamed and the testing is also completed.
Adding Screenshots for your reference.

Please let know if I should commit changes now or wait for the PR to get merged and then create a new PR ?
Also, do you want me to commit the test file too ?

Screenshots

[mitschabaude]

@Saurabhpatil-dev I'd prefer a new PR. And yes please also commit the test changes. Ideally your next PR would also update most examples in src/examples that currently use assert*

[LuffySama-Dev]

Hi @mitschabaude ,

Created a new PR : #1265

Once this PR get approved and merged I would like to start updating the examples in src/examples .
For it will we need a new issue and can I update the examples 😅??

Thank You😃

[LuffySama-Dev]

HI @mitschabaude ,

As discussed I am starting with changing the example files locally.
I am just going through code and then replacing the deprecated ones with the new. (Not retaining the deprecated one)

Can you please check and let me know if this what you were expecting ?

Here is screenshot:

Thank You 😃

[mitschabaude]

Yes @Saurabhpatil-dev, exactly what I was expecting 💯

[LuffySama-Dev]

Hi @mitschabaude,

Okay. Will start committing them.
Is there anyway I can test them or it's fine?

Thanks 😃

[mitschabaude]

They're tested in CI. You can test them locally with ./run file --bundle

[LuffySama-Dev]

Hi @mitschabaude ,

Testing of the example is in progress.
Just wanted you to check if we need to update the error messages too.
If you will see below we are getting the assertEquals() rather than requireEquals():

Transaction failed with error Field.assertEquals(): 

1. Please let me know if we need to update them and where can I do that ?
2. Should I commit all the example files in new PR ?

Screen Shot for reference:

[mitschabaude]

Hey @LuffySama-Dev no that error doesn't need to be updated, because it's about Field.assertEquals(), which didn't change. The whole point of these changes is to distinguish assertions on Field elements and other provable type from preconditions (which now use require())

[mitschabaude]

2. If you'd commit all examples in a new PR, that would be great!

[LuffySama-Dev]

Hi @mitschabaude ,

As discussed new PR is created #1269.
Can you please help to review and approve ?

Thank You 😃

[mitschabaude]

I think we can call this done! Next step for us is to update zkapp-cli examples and docs

[LuffySama-Dev]

Hi @mitschabaude ,

Okay, can you please assign the issue to me once it is raised ?? I would love to start working on it 😃.
Thank you so much for guiding me through out.

Thank You 🙌