feat!: remove offset from decodeLog and decodeFunctionResult methods

[nedsalk]

Interface.decodeLog and Interface.decodeFunctionResult are top-level functions that don't need to return a data offset because you pass in all the data and the returned offset is always the length of that data. If somebody wants to get the offset value after this change, they can just call data.length.

const theInterface = new Interface(...);

// previously
const [decodedLog, offset] = theInterface.decodeLog(data, logId);
// now
const decodedLog = theInterface.decodeLog(data, logId);

// previously
const [decodedFnResult, offset] = theInterface.decodeFunctionResult(fnName, data);
// now
const decodedFnResult = theInterface.decodeFunctionResult(fnName, data);

I tagged it as a feat because it improves the DX of the methods.

Supersedes #2308

[fuel-service-user]

✨ A PR has been created under the rc-2514 tag on fuels-wallet repo.
FuelLabs/fuels-wallet#1374

[petertonysmith94]

Could we highlight the breaking changes in the description please?

Wrong checkbox

[github-actions]

Coverage Report:

Lines	Branches	Functions	Statements
45.59%(+0%)	38.88%(+0%)	42.23%(+0%)	45.37%(+0%)

Changed Files:

Coverage values did not change👌.

[arboleya]

Are there downsides to removing the offset property?

Can someone be looking for the offset in the future?

[nedsalk]

@arboleya the offset value is equal to data.length so people can always get it that way. I updated the description to be more explicit about this.

[arboleya]

Pattern

Could we be breaking the patterns already in use?

fuels-ts/packages/transactions/src/coders/transaction.ts

Lines 104 to 133 in e165e37

	let decoded;
	let o = offset;
	[decoded, o] = new BigNumberCoder('u64').decode(data, o);
	const scriptGasLimit = decoded;
	[decoded, o] = new B256Coder().decode(data, o);
	const receiptsRoot = decoded;
	[decoded, o] = new BigNumberCoder('u64').decode(data, o);
	const scriptLength = decoded;
	[decoded, o] = new BigNumberCoder('u64').decode(data, o);
	const scriptDataLength = decoded;
	[decoded, o] = new NumberCoder('u32', { padToWordSize: true }).decode(data, o);
	const policyTypes = decoded;
	[decoded, o] = new NumberCoder('u16', { padToWordSize: true }).decode(data, o);
	const inputsCount = decoded;
	[decoded, o] = new NumberCoder('u16', { padToWordSize: true }).decode(data, o);
	const outputsCount = decoded;
	[decoded, o] = new NumberCoder('u16', { padToWordSize: true }).decode(data, o);
	const witnessesCount = decoded;
	[decoded, o] = new ByteArrayCoder(scriptLength.toNumber()).decode(data, o);
	const script = decoded;
	[decoded, o] = new ByteArrayCoder(scriptDataLength.toNumber()).decode(data, o);
	const scriptData = decoded;
	[decoded, o] = new PoliciesCoder().decode(data, o, policyTypes);
	const policies = decoded;
	[decoded, o] = new ArrayCoder(new InputCoder(), inputsCount).decode(data, o);
	const inputs = decoded;
	[decoded, o] = new ArrayCoder(new OutputCoder(), outputsCount).decode(data, o);
	const outputs = decoded;
	[decoded, o] = new ArrayCoder(new WitnessCoder(), witnessesCount).decode(data, o);
	const witnesses = decoded;

Exceptions

Also, it seems some Coders have non-obvious ways of computing the offset:

fuels-ts/packages/abi-coder/src/encoding/coders/B256Coder.ts

Line 43 in e165e37

return [toHex(bytes, 32), offset + 32];

fuels-ts/packages/abi-coder/src/encoding/coders/VecCoder.ts

Lines 63 to 69 in e165e37

	let newOffset = offsetAndLength;
	const chunks = [];
	for (let i = 0; i < length; i++) {
	const [decoded, optionOffset] = this.coder.decode(data, newOffset);
	chunks.push(decoded);
	newOffset = optionOffset;
	}

API Preferences

Regarding personal preference, I'd go with Objects instead of Arrays.

But only if we'd change the pattern everywhere, which is not the case.

const { decodedLog, offset } = theInterface.decodeLog(data, logId);
// vs
const [decodedLog, offset] = theInterface.decodeLog(data, logId);

Conclusion

I'm not sure if the breaking change is worth it.

The change is subtle, but the latter hides information.

const [decodedLog] = theInterface.decodeLog(data, logId);
// vs
const decodedLog = theInterface.decodeLog(data, logId);

As for if this is a good or a bad thing, I'll let others chime in.

[nedsalk]

@arboleya you're right, introducing this would break the pattern present in other coders; I didn't think of that. If I remember correctly, there was talk on the offsite around not needing the offset value across the board on all our coders and that this would be considered in #1795.

The argument for merging it right now is that this is a high level API relative to the coders and we can get the DX benefits of it right now because the offset value is never needed when using these methods. The counter-argument is that API consistency is more important and that it should be done in tandem with #1795.

In the end, I agree with your uncertainty

I'm not sure if the breaking change is worth it.

and I'll close this PR. I've added a comment on #1795 linking to this PR.