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

Change ed19 to work with arbitrary-length messages #795

Merged
merged 49 commits into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
41d1d50
Add blob tx
Dentosal Jun 20, 2024
34a23f8
Add some tests
Dentosal Jun 20, 2024
3f233a3
Add opcodes for blob storage access
Dentosal Jun 21, 2024
f47cddd
Freeze stack on DLDC instead of copying it over the data
Dentosal Jun 24, 2024
296e307
Add internal context test cases
Dentosal Jun 24, 2024
66023ef
Merge branch 'master' into dento/blob-tx
Dentosal Jun 24, 2024
d62708e
Add changelog entry
Dentosal Jun 24, 2024
9f84f36
Remove bldc. Allow executable stack.
Dentosal Jun 24, 2024
c316e33
Merge RunResult helper type with AluResult
Dentosal Jun 24, 2024
04e97f1
Move blob tx data to witness like other txs do
Dentosal Jun 26, 2024
56c8f84
Fixes for fuel-core
Dentosal Jun 26, 2024
586ece5
Merge branch 'master' into dento/blob-tx
Dentosal Jun 26, 2024
cc55e94
Merge RunResult types after branch merge
Dentosal Jun 26, 2024
7c0a013
Fix no_std imports
Dentosal Jun 26, 2024
08041d8
Add blob id validation rule
Dentosal Jun 26, 2024
98bdbc8
Add base_asset_id to blob::CheckedMetadata
Dentosal Jun 26, 2024
75e3149
Use LDC with mode argument instead of having separate blob instructions
Dentosal Jul 2, 2024
2b39ae9
Restore BSIZ and BLDD instructions
Dentosal Jul 2, 2024
0e0e79c
Merge branch 'master' into dento/blob-tx
Dentosal Jul 2, 2024
862687e
Merge branch 'master' into dento/blob-tx
Dentosal Jul 11, 2024
578b6b6
fmt
Dentosal Jul 11, 2024
d42c469
Update doc comment (PR feedback)
Dentosal Jul 15, 2024
3570b60
Improve test case names (pr feedback)
Dentosal Jul 22, 2024
2220fb5
Apply PR review suggestions
Dentosal Jul 22, 2024
476f2f4
Fix the serialization for the blob
xgreenx Jul 23, 2024
509cb57
Merge remote-tracking branch 'origin/dento/blob-tx' into dento/blob-tx
xgreenx Jul 23, 2024
a977954
Fix the serialization for the blob
xgreenx Jul 23, 2024
443f12d
Make clippy happy
xgreenx Jul 23, 2024
3a9f032
Add offset and encoding tests
Dentosal Jul 23, 2024
c844fe4
Add as_blob(_mut) for tx
Dentosal Jul 23, 2024
519839f
fix clippy
Dentosal Jul 23, 2024
fd48197
Preparation for the `ed19` modifications
xgreenx Jul 24, 2024
24e3b0d
Updated CHANGELOG.md
xgreenx Jul 24, 2024
f7f70ae
Updated comment
xgreenx Jul 24, 2024
9c945c7
Actually implement the opcode changes
Dentosal Jul 26, 2024
7e2d619
Update changelog
Dentosal Jul 26, 2024
d741a42
Change the gas cost approximation
Dentosal Jul 26, 2024
f964db6
Fix some tests (PR comments)
Dentosal Jul 26, 2024
6de7874
Add a test case for zero length mapping to 32
Dentosal Jul 26, 2024
1bfa69b
Merge branch 'master' into feature/ed19-with-dynamic-price
Dentosal Jul 26, 2024
4f0a032
Remove extra code added in merge
Dentosal Jul 26, 2024
e9a87d4
Fix more issues of merge
Dentosal Jul 26, 2024
75ab1a1
And even more
Dentosal Jul 26, 2024
6e72acc
I though I already removed this one
Dentosal Jul 26, 2024
3bc3a86
Add couple more test cases
Dentosal Jul 26, 2024
6a155a0
Fix no_std imports for tests
Dentosal Jul 26, 2024
1c56802
Remove more incorrect merge changes
Dentosal Jul 27, 2024
eb11817
Rename and clean up crypto opcode unit tests
Dentosal Jul 27, 2024
a884c4c
Fix typo in cfg
Dentosal Jul 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

#### Breaking
- [#780](https://github.com/FuelLabs/fuel-vm/pull/780): Added `Blob` transaction, and `BSIZ` and `BLDD` instructions. Also allows `LDC` to load blobs.
- [#795](https://github.com/FuelLabs/fuel-vm/pull/795): Fixed `ed19` instruction to take variable length message instead of a fixed-length one. Changed the gas cost to be `DependentCost`.

## [Version 0.55.0]

Expand Down
8 changes: 4 additions & 4 deletions fuel-asm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,12 @@ impl_instructions! {
0x3C TR tr [contract_id_addr: RegId amount: RegId asset_id_addr: RegId]
"Transfer coins to a variable output."
0x3D TRO tro [contract_id_addr: RegId output_index: RegId amount: RegId asset_id_addr: RegId]
"The 64-byte public key (x, y) recovered from 64-byte signature on 32-byte message."
"The 64-byte public key (x, y) recovered from 64-byte signature on 32-byte message hash."
0x3E ECK1 eck1 [dst_addr: RegId sig_addr: RegId msg_hash_addr: RegId]
"The 64-byte Secp256r1 public key (x, y) recovered from 64-byte signature on 32-byte message."
"The 64-byte Secp256r1 public key (x, y) recovered from 64-byte signature on 32-byte message hash."
0x3F ECR1 ecr1 [dst_addr: RegId sig_addr: RegId msg_hash_addr: RegId]
"Verify ED25519 public key and signature match a 32-byte message."
0x40 ED19 ed19 [pub_key_addr: RegId sig_addr: RegId msg_hash_addr: RegId]
"Verify ED25519 public key and signature match a message."
0x40 ED19 ed19 [pub_key_addr: RegId sig_addr: RegId msg_addr: RegId msg_len: RegId]
"The keccak-256 hash of a slice."
0x41 K256 k256 [dst_addr: RegId src_addr: RegId len: RegId]
"The SHA-2-256 hash of a slice."
Expand Down
9 changes: 3 additions & 6 deletions fuel-crypto/src/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,20 @@ use fuel_types::{
Bytes64,
};

use crate::{
message::Message,
Error,
};
use crate::Error;

/// Verify a signature against a message digest and a public key.
pub fn verify(
pub_key: &Bytes32,
signature: &Bytes64,
message: &Message,
message: &[u8],
) -> Result<(), Error> {
let signature = Signature::from_bytes(signature);

let pub_key = ed25519_dalek::VerifyingKey::from_bytes(pub_key)
.map_err(|_| Error::InvalidPublicKey)?;

if pub_key.verify_strict(&**message, &signature).is_ok() {
if pub_key.verify_strict(message, &signature).is_ok() {
Ok(())
} else {
Err(Error::InvalidSignature)
Expand Down
34 changes: 22 additions & 12 deletions fuel-tx/src/transaction/consensus_parameters/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,6 @@ impl GasCostsValues {
}
}

pub fn ed19(&self) -> Word {
match self {
GasCostsValues::V1(v1) => v1.ed19,
GasCostsValues::V2(v2) => v2.ed19,
GasCostsValues::V3(v3) => v3.ed19,
GasCostsValues::V4(v4) => v4.ed19,
}
}

pub fn eq_(&self) -> Word {
match self {
GasCostsValues::V1(v1) => v1.eq,
Expand Down Expand Up @@ -945,6 +936,24 @@ impl GasCostsValues {
}
}

pub fn ed19(&self) -> DependentCost {
match self {
GasCostsValues::V1(v1) => DependentCost::HeavyOperation {
base: v1.ed19,
gas_per_unit: 0,
},
GasCostsValues::V2(v2) => DependentCost::HeavyOperation {
base: v2.ed19,
gas_per_unit: 0,
},
GasCostsValues::V3(v3) => DependentCost::HeavyOperation {
base: v3.ed19,
gas_per_unit: 0,
},
GasCostsValues::V4(v4) => v4.ed19,
Copy link
Member

Choose a reason for hiding this comment

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

Why was it HeavyOperation before, but LightOperation now?

Copy link
Member

Choose a reason for hiding this comment

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

Probably just an oversight. Brandon was wonderin which one it should be, and changing it seemed to make sense. #795 (comment)

}
}

pub fn k256(&self) -> DependentCost {
match self {
GasCostsValues::V1(v1) => v1.k256,
Expand Down Expand Up @@ -1513,6 +1522,7 @@ pub struct GasCostsValuesV3 {
/// Gas costs for every op.
/// The difference with [`GasCostsValuesV3`]:
/// - Added `bsiz`, `bldd` instructions
/// - Changed `ed19` to be `DependentCost`
#[allow(missing_docs)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
#[serde(default = "GasCostsValuesV4::unit")]
Expand All @@ -1531,7 +1541,6 @@ pub struct GasCostsValuesV4 {
pub divi: Word,
pub eck1: Word,
pub ecr1: Word,
pub ed19: Word,
pub eq: Word,
pub exp: Word,
pub expi: Word,
Expand Down Expand Up @@ -1618,6 +1627,7 @@ pub struct GasCostsValuesV4 {
pub ccp: DependentCost,
pub croo: DependentCost,
pub csiz: DependentCost,
pub ed19: DependentCost,
pub k256: DependentCost,
pub ldc: DependentCost,
pub logd: DependentCost,
Expand Down Expand Up @@ -2428,7 +2438,6 @@ impl GasCostsValuesV4 {
divi: 0,
eck1: 0,
ecr1: 0,
ed19: 0,
eq: 0,
exp: 0,
expi: 0,
Expand Down Expand Up @@ -2509,6 +2518,7 @@ impl GasCostsValuesV4 {
ccp: DependentCost::free(),
croo: DependentCost::free(),
csiz: DependentCost::free(),
ed19: DependentCost::free(),
k256: DependentCost::free(),
ldc: DependentCost::free(),
logd: DependentCost::free(),
Expand Down Expand Up @@ -2549,7 +2559,6 @@ impl GasCostsValuesV4 {
divi: 1,
eck1: 1,
ecr1: 1,
ed19: 1,
eq: 1,
exp: 1,
expi: 1,
Expand Down Expand Up @@ -2630,6 +2639,7 @@ impl GasCostsValuesV4 {
ccp: DependentCost::unit(),
croo: DependentCost::unit(),
csiz: DependentCost::unit(),
ed19: DependentCost::unit(),
k256: DependentCost::unit(),
ldc: DependentCost::unit(),
logd: DependentCost::unit(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub fn default_gas_costs() -> GasCostsValues {
divi: 1,
eck1: 951,
ecr1: 3000,
ed19: 3000,
eq: 1,
exp: 1,
expi: 1,
Expand Down Expand Up @@ -102,6 +101,10 @@ pub fn default_gas_costs() -> GasCostsValues {
base: 2,
units_per_gas: 214,
},
ed19: DependentCost::LightOperation {
base: 3000,
units_per_gas: 214,
},
k256: DependentCost::LightOperation {
base: 11,
units_per_gas: 214,
Expand Down
9 changes: 5 additions & 4 deletions fuel-vm/src/interpreter/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ where
a: Word,
b: Word,
c: Word,
len: Word,
) -> SimpleResult<()> {
let (SystemRegisters { err, pc, .. }, _) = split_registers(&mut self.registers);
ed25519_verify(self.memory.as_mut(), err, pc, a, b, c)
ed25519_verify(self.memory.as_mut(), err, pc, a, b, c, len)
}

pub(crate) fn keccak256(&mut self, a: Word, b: Word, c: Word) -> SimpleResult<()> {
Expand Down Expand Up @@ -155,13 +156,13 @@ pub(crate) fn ed25519_verify(
a: Word,
b: Word,
c: Word,
len: Word,
) -> SimpleResult<()> {
let pub_key = Bytes32::from(memory.read_bytes(a)?);
let sig = Bytes64::from(memory.read_bytes(b)?);
let msg = Bytes32::from(memory.read_bytes(c)?);
let message = Message::from_bytes_ref(&msg);
let msg = memory.read(c, len)?;

if fuel_crypto::ed25519::verify(&pub_key, &sig, message).is_ok() {
if fuel_crypto::ed25519::verify(&pub_key, &sig, msg).is_ok() {
clear_err(err);
} else {
set_err(err);
Expand Down
19 changes: 11 additions & 8 deletions fuel-vm/src/interpreter/crypto/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use alloc::vec;
use fuel_crypto::SecretKey;
use rand::{
rngs::StdRng,
RngCore,
SeedableRng,
};

Expand Down Expand Up @@ -110,21 +111,22 @@ fn test_verify_ed25519() -> SimpleResult<()> {
let mut err = 0;
let mut pc = 4;

let sig_address = 0;
let msg_address = 64;
let pubkey_address = 64 + 32;
let pubkey_address = 0;
let sig_address = pubkey_address + 32;
let msg_address = sig_address + 64;

let mut rng = rand::rngs::OsRng;
let signing_key = ed25519_dalek::SigningKey::generate(&mut rng);

let message = Message::new([3u8; 100]);
let signature = signing_key.sign(&*message);
let mut message = [0u8; 100];
rng.fill_bytes(&mut message);
let signature = signing_key.sign(&message);

memory[sig_address..sig_address + Signature::LEN]
.copy_from_slice(&signature.to_bytes());
memory[msg_address..msg_address + Message::LEN].copy_from_slice(message.as_ref());
memory[pubkey_address..pubkey_address + Bytes32::LEN]
.copy_from_slice(signing_key.verifying_key().as_ref());
memory[sig_address..sig_address + Signature::LEN]
.copy_from_slice(&signature.to_bytes());
memory[msg_address..msg_address + message.len()].copy_from_slice(message.as_ref());

ed25519_verify(
&mut memory,
Expand All @@ -133,6 +135,7 @@ fn test_verify_ed25519() -> SimpleResult<()> {
pubkey_address as Word,
sig_address as Word,
msg_address as Word,
message.len() as Word,
)?;
assert_eq!(pc, 8);
assert_eq!(err, 0);
Expand Down
15 changes: 11 additions & 4 deletions fuel-vm/src/interpreter/executors/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ where
))
})?,
);
if pc < self.registers[RegId::IS] || pc >= self.registers[RegId::SSP] {
if pc < self.registers[RegId::IS] || pc >= self.registers[RegId::SP] {
Copy link
Member

Choose a reason for hiding this comment

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

Why did this get changed to SP from SSP?

Copy link
Member

Choose a reason for hiding this comment

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

The PR originially included changes from an old version of the Blob TX PR. Git merged this part incorrectly and I missed it when reviewing the code myself. Removed in 1c56802.

Copy link
Member

Choose a reason for hiding this comment

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

I re-reviewed myself as well to make sure nothing else was missed.

return Err(InterpreterError::PanicInstruction(PanicInstruction::error(
PanicReason::MemoryNotExecutable,
instruction,
Expand Down Expand Up @@ -845,9 +845,16 @@ where
}

Instruction::ED19(ed19) => {
self.gas_charge(self.gas_costs().ed19())?;
let (a, b, c) = ed19.unpack();
self.ed25519_verify(r!(a), r!(b), r!(c))?;
let (a, b, c, len) = ed19.unpack();
let mut len = r!(len);

// Backwards compatibility with old contracts
if len == 0 {
len = 32;
}

self.dependent_gas_charge(self.gas_costs().ed19(), len)?;
self.ed25519_verify(r!(a), r!(b), r!(c), len)?;
}

Instruction::K256(k256) => {
Expand Down
Loading
Loading